Add sequence numbers to game state

This oopses on output dependency violations, e.g. two threads doing a
read-modify-write without synchronization, or the one thread nesting
several read-modify-writes.  Such bugs are difficult to spot, and tend
to be abusable.  I figure we have quite a few of them.

New struct emptypedstr member seqno.  Make sure all members of unit
empobj_storage share it.  Initialize it in files: main() and
file_sct_init().  Set it in ef_blank() and new ef_set_uid() by calling
new get_seqno().  Use ef_set_uid() when copying objects: swaps(),
doland(), doship(), doplane(), dounit(), delete_old_news().  Step it
in ef_write() by calling new new_seqno().

Factor do_read() out of fillcache() to make it available for
get_seqno().
This commit is contained in:
Markus Armbruster 2008-05-17 22:44:00 +02:00
parent 087c0aae36
commit 536ef0b0a2
20 changed files with 134 additions and 19 deletions

View file

@ -44,6 +44,7 @@ struct comstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short com_uid; short com_uid;
unsigned com_seqno;
time_t com_timestamp; time_t com_timestamp;
natid com_owner; natid com_owner;
/* end of part matching struct empobj */ /* end of part matching struct empobj */

View file

@ -57,6 +57,7 @@ struct empobj {
*/ */
short ef_type; short ef_type;
short uid; short uid;
unsigned seqno;
time_t timestamp; time_t timestamp;
/* end of part matching struct emptypedstr */ /* end of part matching struct emptypedstr */
natid own; /* valid if EFF_OWNER is in table's flags */ natid own; /* valid if EFF_OWNER is in table's flags */

View file

@ -67,6 +67,7 @@ struct empfile {
struct emptypedstr { struct emptypedstr {
short ef_type; short ef_type;
short uid; short uid;
unsigned seqno;
time_t timestamp; time_t timestamp;
}; };
@ -189,6 +190,7 @@ extern int ef_close(int);
extern int ef_flush(int); extern int ef_flush(int);
extern void ef_blank(int, int, void *); extern void ef_blank(int, int, void *);
extern int ef_write(int, int, void *); extern int ef_write(int, int, void *);
extern void ef_set_uid(int, void *, int);
extern int ef_extend(int, int); extern int ef_extend(int, int);
extern int ef_ensure_space(int, int, int); extern int ef_ensure_space(int, int, int);
extern int ef_truncate(int, int); extern int ef_truncate(int, int);

View file

@ -40,6 +40,7 @@ struct gamestr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short game_uid; short game_uid;
unsigned game_seqno;
time_t game_timestamp; time_t game_timestamp;
/* end of part matching struct empobj */ /* end of part matching struct empobj */
char game_upd_disable; /* updates disabled? */ char game_upd_disable; /* updates disabled? */

View file

@ -51,6 +51,7 @@ struct lndstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short lnd_uid; /* unit id (land unit) */ short lnd_uid; /* unit id (land unit) */
unsigned lnd_seqno;
time_t lnd_timestamp; /* Last time this unit was touched */ time_t lnd_timestamp; /* Last time this unit was touched */
natid lnd_own; /* owner's country num */ natid lnd_own; /* owner's country num */
coord lnd_x; /* x location in abs coords */ coord lnd_x; /* x location in abs coords */

View file

@ -44,6 +44,7 @@ struct lonstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short l_uid; short l_uid;
unsigned l_seqno;
time_t l_timestamp; time_t l_timestamp;
/* end of part matching struct empobj */ /* end of part matching struct empobj */
natid l_loner; /* loan shark */ natid l_loner; /* loan shark */

View file

@ -41,6 +41,7 @@ struct loststr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
int lost_uid; int lost_uid;
unsigned lost_seqno;
time_t lost_timestamp; /* When it was lost */ time_t lost_timestamp; /* When it was lost */
natid lost_owner; /* Who lost it */ natid lost_owner; /* Who lost it */
/* end of part matching struct empobj */ /* end of part matching struct empobj */

View file

@ -70,6 +70,7 @@ struct realmstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short r_uid; /* realm table index */ short r_uid; /* realm table index */
unsigned r_seqno;
time_t r_timestamp; /* Last time this realm was touched */ time_t r_timestamp; /* Last time this realm was touched */
natid r_cnum; /* country number */ natid r_cnum; /* country number */
/* end of part matching struct empobj */ /* end of part matching struct empobj */
@ -82,6 +83,7 @@ struct natstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short nat_uid; /* equals nat_cnum */ short nat_uid; /* equals nat_cnum */
unsigned nat_seqno;
time_t nat_timestamp; time_t nat_timestamp;
natid nat_cnum; /* our country number */ natid nat_cnum; /* our country number */
/* end of part matching struct empobj */ /* end of part matching struct empobj */

View file

@ -48,6 +48,7 @@ struct nwsstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short nws_uid; short nws_uid;
unsigned nws_seqno;
time_t nws_timestamp; time_t nws_timestamp;
/* end of part matching struct empobj */ /* end of part matching struct empobj */
natid nws_ano; /* "actor" country # */ natid nws_ano; /* "actor" country # */

View file

@ -44,6 +44,7 @@ struct nukstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short nuk_uid; short nuk_uid;
unsigned nuk_seqno;
time_t nuk_timestamp; /* Last time this nuke was touched */ time_t nuk_timestamp; /* Last time this nuke was touched */
natid nuk_own; natid nuk_own;
coord nuk_x, nuk_y; /* current loc of device */ coord nuk_x, nuk_y; /* current loc of device */

View file

@ -48,6 +48,7 @@ struct plnstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short pln_uid; /* plane unit id */ short pln_uid; /* plane unit id */
unsigned pln_seqno;
time_t pln_timestamp; /* Last time this plane was touched */ time_t pln_timestamp; /* Last time this plane was touched */
natid pln_own; /* owning country */ natid pln_own; /* owning country */
coord pln_x; /* plane x-y */ coord pln_x; /* plane x-y */

View file

@ -46,6 +46,7 @@ struct sctstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short sct_uid; /* equals XYOFFSET(sct_x, sct_y) */ short sct_uid; /* equals XYOFFSET(sct_x, sct_y) */
unsigned sct_seqno;
time_t sct_timestamp; /* Last time this sector was written to */ time_t sct_timestamp; /* Last time this sector was written to */
natid sct_own; /* owner's country num */ natid sct_own; /* owner's country num */
coord sct_x; /* x coord of sector */ coord sct_x; /* x coord of sector */

View file

@ -65,6 +65,7 @@ struct shpstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short shp_uid; /* unit id (ship #) */ short shp_uid; /* unit id (ship #) */
unsigned shp_seqno;
time_t shp_timestamp; /* Last time this ship was touched. */ time_t shp_timestamp; /* Last time this ship was touched. */
natid shp_own; /* owner's country num */ natid shp_own; /* owner's country num */
coord shp_x; /* x location in abs coords */ coord shp_x; /* x location in abs coords */

View file

@ -44,6 +44,7 @@ struct trdstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short trd_uid; short trd_uid;
unsigned trd_seqno;
time_t trd_timestamp; time_t trd_timestamp;
natid trd_owner; natid trd_owner;
/* end of part matching struct empobj */ /* end of part matching struct empobj */

View file

@ -41,6 +41,7 @@ struct trtstr {
/* initial part must match struct empobj */ /* initial part must match struct empobj */
short ef_type; short ef_type;
short trt_uid; short trt_uid;
unsigned trt_seqno;
time_t trt_timestamp; time_t trt_timestamp;
/* end of part matching struct empobj */ /* end of part matching struct empobj */
natid trt_cna; /* proposer */ natid trt_cna; /* proposer */

View file

@ -588,7 +588,7 @@ doland(char op, int arg, char *p, struct sctstr *sect)
return RET_SYN; return RET_SYN;
sect->sct_x = newx; sect->sct_x = newx;
sect->sct_y = newy; sect->sct_y = newy;
sect->sct_uid = XYOFFSET(newx, newy); ef_set_uid(EF_SECTOR, &sect, XYOFFSET(newx, newy));
break; break;
case 'D': case 'D':
if (!sarg_xy(p, &newx, &newy)) if (!sarg_xy(p, &newx, &newy))
@ -771,7 +771,7 @@ doship(char op, int arg, char *p, struct shpstr *ship)
ship->shp_rflags = arg; ship->shp_rflags = arg;
break; break;
case 'U': case 'U':
ship->shp_uid = arg; ef_set_uid(EF_SHIP, ship, arg);
break; break;
case 'O': case 'O':
if (ship->shp_own) if (ship->shp_own)
@ -875,7 +875,7 @@ dounit(char op, int arg, char *p, struct lndstr *land)
land->lnd_land = arg; land->lnd_land = arg;
break; break;
case 'U': case 'U':
land->lnd_uid = arg; ef_set_uid(EF_LAND, land, arg);
break; break;
case 'O': case 'O':
if (land->lnd_own) if (land->lnd_own)
@ -998,7 +998,7 @@ doplane(char op, int arg, char *p, struct plnstr *plane)
plane->pln_nuketype = arg; plane->pln_nuketype = arg;
break; break;
case 'U': case 'U':
plane->pln_uid = arg; ef_set_uid(EF_PLANE, plane, arg);
break; break;
case 'l': case 'l':
if (!sarg_xy(p, &newx, &newy)) if (!sarg_xy(p, &newx, &newy))

View file

@ -63,14 +63,14 @@ swaps(void)
/* change the location of secta to that of sectb */ /* change the location of secta to that of sectb */
secta.sct_x = sectb.sct_x; secta.sct_x = sectb.sct_x;
secta.sct_y = sectb.sct_y; secta.sct_y = sectb.sct_y;
secta.sct_uid = sectb.sct_uid; ef_set_uid(EF_SECTOR, &secta, sectb.sct_uid);
secta.sct_dist_x = sectb.sct_x; secta.sct_dist_x = sectb.sct_x;
secta.sct_dist_y = sectb.sct_y; secta.sct_dist_y = sectb.sct_y;
secta.sct_coastal = sectb.sct_coastal; secta.sct_coastal = sectb.sct_coastal;
/* change the location of sectb to where secta was */ /* change the location of sectb to where secta was */
sectb.sct_x = tmp.sct_x; sectb.sct_x = tmp.sct_x;
sectb.sct_y = tmp.sct_y; sectb.sct_y = tmp.sct_y;
sectb.sct_uid = tmp.sct_uid; ef_set_uid(EF_SECTOR, &sectb, tmp.sct_uid);
sectb.sct_dist_x = tmp.sct_x; sectb.sct_dist_x = tmp.sct_x;
sectb.sct_dist_y = tmp.sct_y; sectb.sct_dist_y = tmp.sct_y;
sectb.sct_coastal = tmp.sct_coastal; sectb.sct_coastal = tmp.sct_coastal;

View file

@ -48,7 +48,10 @@
static int ef_realloc_cache(struct empfile *, int); static int ef_realloc_cache(struct empfile *, int);
static int fillcache(struct empfile *, int); static int fillcache(struct empfile *, int);
static int do_read(struct empfile *, void *, int, int);
static int do_write(struct empfile *, void *, int, int); static int do_write(struct empfile *, void *, int, int);
static unsigned get_seqno(struct empfile *, int);
static void new_seqno(struct empfile *, void *);
static void do_blank(struct empfile *, void *, int, int); static void do_blank(struct empfile *, void *, int, int);
/* /*
@ -308,11 +311,28 @@ ef_read(int type, int id, void *into)
*/ */
static int static int
fillcache(struct empfile *ep, int id) fillcache(struct empfile *ep, int id)
{
int ret;
if (CANT_HAPPEN(!ep->cache))
return -1;
ret = do_read(ep, ep->cache, id, MIN(ep->csize, ep->fids - id));
if (ret >= 0) {
/* cache changed */
ep->baseid = id;
ep->cids = ret;
}
return ret;
}
static int
do_read(struct empfile *ep, void *buf, int id, int count)
{ {
int n, ret; int n, ret;
char *p; char *p;
if (CANT_HAPPEN(ep->fd < 0 || !ep->cache)) if (CANT_HAPPEN(ep->fd < 0 || id < 0 || count < 0))
return -1; return -1;
if (lseek(ep->fd, id * ep->size, SEEK_SET) == (off_t)-1) { if (lseek(ep->fd, id * ep->size, SEEK_SET) == (off_t)-1) {
@ -321,21 +341,21 @@ fillcache(struct empfile *ep, int id)
return -1; return -1;
} }
p = ep->cache; p = buf;
n = MIN(ep->csize, ep->fids - id) * ep->size; n = count * ep->size;
while (n > 0) { while (n > 0) {
ret = read(ep->fd, p, n); ret = read(ep->fd, p, n);
if (ret < 0) { if (ret < 0) {
if (errno != EINTR) { if (errno != EINTR) {
logerror("Error reading %s elt %d (%s)", logerror("Error reading %s elt %d (%s)",
ep->file, ep->file,
id + (int)((p - ep->cache) / ep->size), id + (int)((p - (char *)buf) / ep->size),
strerror(errno)); strerror(errno));
break; break;
} }
} else if (ret == 0) { } else if (ret == 0) {
logerror("Unexpected EOF reading %s elt %d", logerror("Unexpected EOF reading %s elt %d",
ep->file, id + (int)((p - ep->cache) / ep->size)); ep->file, id + (int)((p - (char *)buf) / ep->size));
break; break;
} else { } else {
p += ret; p += ret;
@ -343,12 +363,7 @@ fillcache(struct empfile *ep, int id)
} }
} }
if (p == ep->cache) return (p - (char *)buf) / ep->size;
return -1; /* nothing read, old cache still ok */
ep->baseid = id;
ep->cids = (p - ep->cache) / ep->size;
return ep->cids;
} }
/* /*
@ -440,6 +455,7 @@ ef_write(int type, int id, void *from)
ep->prewrite(id, from); ep->prewrite(id, from);
if (CANT_HAPPEN((ep->flags & EFF_MEM) ? id >= ep->fids : id > ep->fids)) if (CANT_HAPPEN((ep->flags & EFF_MEM) ? id >= ep->fids : id > ep->fids))
return 0; /* not implemented */ return 0; /* not implemented */
new_seqno(ep, from);
if (ep->fd >= 0) { if (ep->fd >= 0) {
if (do_write(ep, from, id, 1) < 0) if (do_write(ep, from, id, 1) < 0)
return 0; return 0;
@ -457,6 +473,77 @@ ef_write(int type, int id, void *from)
return 1; return 1;
} }
/*
* Change element id.
* BUF is an element of table TYPE.
* ID is its new element ID.
* If table is EFF_TYPED, change id and sequence number stored in BUF.
* Else do nothing.
*/
void
ef_set_uid(int type, void *buf, int uid)
{
struct emptypedstr *elt;
struct empfile *ep;
if (ef_check(type) < 0)
return;
ep = &empfile[type];
if (!(ep->flags & EFF_TYPED))
return;
elt = buf;
if (elt->uid == uid)
return;
elt->uid = uid;
elt->seqno = get_seqno(ep, uid);
}
/*
* Return sequence number of element ID in table EP.
* Return zero if table is not EFF_TYPED (it has no sequence number
* then).
*/
static unsigned
get_seqno(struct empfile *ep, int id)
{
struct emptypedstr *elt;
if (!(ep->flags & EFF_TYPED))
return 0;
if (id < 0 || id >= ep->fids)
return 0;
if (id >= ep->baseid && id < ep->baseid + ep->cids)
elt = (void *)(ep->cache + (id - ep->baseid) * ep->size);
else {
/* need a buffer, steal last cache slot */
if (ep->cids == ep->csize)
ep->cids--;
elt = (void *)(ep->cache + ep->cids * ep->size);
if (do_read(ep, elt, id, 1) < 0)
return 0; /* deep trouble */
}
return elt->seqno;
}
/*
* Increment sequence number in BUF, which is about to be written to EP.
* Do nothing if table is not EFF_TYPED (it has no sequence number
* then).
*/
static void
new_seqno(struct empfile *ep, void *buf)
{
struct emptypedstr *elt = buf;
unsigned old_seqno;
if (!(ep->flags & EFF_TYPED))
return;
old_seqno = get_seqno(ep, elt->uid);
if (CANT_HAPPEN(old_seqno != elt->seqno))
old_seqno = MAX(old_seqno, elt->seqno);
elt->seqno = old_seqno + 1;
}
/* /*
* Extend table TYPE by COUNT elements. * Extend table TYPE by COUNT elements.
* Any pointers obtained from ef_ptr() become invalid. * Any pointers obtained from ef_ptr() become invalid.
@ -518,9 +605,17 @@ ef_extend(int type, int count)
void void
ef_blank(int type, int id, void *buf) ef_blank(int type, int id, void *buf)
{ {
struct empfile *ep;
struct emptypedstr *elt;
if (ef_check(type) < 0) if (ef_check(type) < 0)
return; return;
do_blank(&empfile[type], buf, id, 1); ep = &empfile[type];
do_blank(ep, buf, id, 1);
if (ep->flags & EFF_TYPED) {
elt = buf;
elt->seqno = get_seqno(ep, elt->uid);
}
} }
/* /*

View file

@ -111,7 +111,7 @@ delete_old_news(void)
for (j = 0; getnews(i + j, &news); j++) { for (j = 0; getnews(i + j, &news); j++) {
if (news.nws_vrb == 0) if (news.nws_vrb == 0)
break; break;
news.nws_uid = j; ef_set_uid(EF_NEWS, &news, j);
putnews(j, &news); putnews(j, &news);
} }
CANT_HAPPEN(i + j != news_tail); CANT_HAPPEN(i + j != news_tail);

View file

@ -155,6 +155,7 @@ main(int argc, char *argv[])
for (i = 1; i < MAXNOC; i++) { for (i = 1; i < MAXNOC; i++) {
nat.ef_type = EF_NATION; nat.ef_type = EF_NATION;
nat.nat_cnum = nat.nat_uid = i; nat.nat_cnum = nat.nat_uid = i;
nat.nat_seqno = 0;
putnat((&nat)); putnat((&nat));
} }
memset(&realm, 0, sizeof(realm)); memset(&realm, 0, sizeof(realm));
@ -164,6 +165,7 @@ main(int argc, char *argv[])
for (j = 0; j < MAXNOR; j++) { for (j = 0; j < MAXNOR; j++) {
realm.r_realm = j; realm.r_realm = j;
realm.r_uid = (i * MAXNOR) + j; realm.r_uid = (i * MAXNOR) + j;
realm.r_seqno = 0;
putrealm(&realm); putrealm(&realm);
} }
} }
@ -209,6 +211,7 @@ file_sct_init(coord x, coord y, struct sctstr *ptr)
sp->ef_type = EF_SECTOR; sp->ef_type = EF_SECTOR;
sp->sct_uid = XYOFFSET(x, y); sp->sct_uid = XYOFFSET(x, y);
sp->sct_seqno = 0;
sp->sct_x = x; sp->sct_x = x;
sp->sct_y = y; sp->sct_y = y;
sp->sct_dist_x = x; sp->sct_dist_x = x;