From 20b31defe7f0dbf68ce48c09f464624fa4090554 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@pond.sub.org>
Date: Sun, 1 Mar 2009 18:56:41 +0100
Subject: [PATCH] Generation numbers to catch write back of stale copies

Oops when a stale copy is written back, i.e. the processor was yielded
since the copy was made.  Such bugs are difficult to spot.  Sequence
numbers catch them when they do actual harm (they also catch different
bugs).  Generation numbers catch them even when they don't.

New ef_generation to count generations.  Call new ef_make_stale() to
step it whenever the processor may be yielded.

New struct emptypedstr member generation.  Make sure all members of
unit empobj_storage share it.  It is only used in copies; its value on
disk and in the cache is meaningless.  Set it to ef_generation by
calling new ef_mark_fresh() when making copies in ef_read() and
ef_blank().  Do the same in obj_changed() to make check_sect_ok() &
friends freshen their argument when it is unchanged.  Copies with
generation other than ef_generation are stale.

Oops in ef_write() when a stale copy is written back.
---
 include/commodity.h     |  1 +
 include/empobj.h        |  1 +
 include/file.h          |  3 +++
 include/game.h          |  1 +
 include/land.h          |  1 +
 include/loan.h          |  1 +
 include/lost.h          |  1 +
 include/nat.h           |  2 ++
 include/news.h          |  1 +
 include/nuke.h          |  1 +
 include/plane.h         |  1 +
 include/sect.h          |  1 +
 include/ship.h          |  1 +
 include/trade.h         |  1 +
 include/treaty.h        |  1 +
 src/lib/common/file.c   | 40 ++++++++++++++++++++++++++++++++++++++--
 src/lib/empthread/io.c  | 12 ++++++++++++
 src/lib/empthread/lwp.c |  9 +++++++++
 src/lib/subs/check.c    |  6 +++++-
 19 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/include/commodity.h b/include/commodity.h
index e35457851..38ef09906 100644
--- a/include/commodity.h
+++ b/include/commodity.h
@@ -45,6 +45,7 @@ struct comstr {
     short ef_type;
     short com_uid;
     unsigned com_seqno;
+    unsigned com_generation;
     time_t com_timestamp;
     natid com_owner;
     /* end of part matching struct empobj */
diff --git a/include/empobj.h b/include/empobj.h
index 713f2f4a5..3906c13a6 100644
--- a/include/empobj.h
+++ b/include/empobj.h
@@ -58,6 +58,7 @@ struct empobj {
     short ef_type;
     short uid;
     unsigned seqno;
+    unsigned generation;
     time_t timestamp;
     /* end of part matching struct emptypedstr */
     natid own;		/* valid if EFF_OWNER is in table's flags */
diff --git a/include/file.h b/include/file.h
index 9aa6f3892..0ffa5b0cf 100644
--- a/include/file.h
+++ b/include/file.h
@@ -88,6 +88,7 @@ struct emptypedstr {
     short ef_type;
     short uid;
     unsigned seqno;
+    unsigned generation;
     time_t timestamp;
 };
 
@@ -203,6 +204,8 @@ enum {
 
 extern struct castr *ef_cadef(int);
 extern int ef_read(int, int, void *);
+extern void ef_make_stale(void);
+extern void ef_mark_fresh(int, void *);
 extern void *ef_ptr(int, int);
 extern char *ef_nameof(int);
 extern time_t ef_mtime(int);
diff --git a/include/game.h b/include/game.h
index 9f77dc0ff..b10426fc5 100644
--- a/include/game.h
+++ b/include/game.h
@@ -41,6 +41,7 @@ struct gamestr {
     short ef_type;
     short game_uid;
     unsigned game_seqno;
+    unsigned game_generation;
     time_t game_timestamp;
     /* end of part matching struct empobj */
     char game_upd_disable;	/* updates disabled? */
diff --git a/include/land.h b/include/land.h
index 6f5232ffc..99f9c301f 100644
--- a/include/land.h
+++ b/include/land.h
@@ -52,6 +52,7 @@ struct lndstr {
     short ef_type;
     short lnd_uid;		/* unit id (land unit) */
     unsigned lnd_seqno;
+    unsigned lnd_generation;
     time_t lnd_timestamp;	/* Last time this unit was touched */
     natid lnd_own;		/* owner's country num */
     coord lnd_x;		/* x location in abs coords */
diff --git a/include/loan.h b/include/loan.h
index f4f900f7c..fbfe15687 100644
--- a/include/loan.h
+++ b/include/loan.h
@@ -45,6 +45,7 @@ struct lonstr {
     short ef_type;
     short l_uid;
     unsigned l_seqno;
+    unsigned l_generation;
     time_t l_timestamp;
     /* end of part matching struct empobj */
     natid l_loner;		/* loan shark */
diff --git a/include/lost.h b/include/lost.h
index d30f8e856..021f26338 100644
--- a/include/lost.h
+++ b/include/lost.h
@@ -42,6 +42,7 @@ struct loststr {
     short ef_type;
     short lost_uid;
     unsigned lost_seqno;
+    unsigned lost_generation;
     time_t lost_timestamp;	/* When it was lost */
     natid lost_owner;		/* Who lost it */
     /* end of part matching struct empobj */
diff --git a/include/nat.h b/include/nat.h
index 5749a1bd0..e4c135f93 100644
--- a/include/nat.h
+++ b/include/nat.h
@@ -73,6 +73,7 @@ struct realmstr {
     short ef_type;
     short r_uid;		/* realm table index */
     unsigned r_seqno;
+    unsigned r_generation;
     time_t r_timestamp;		/* Last time this realm was touched */
     natid r_cnum;		/* country number */
     /* end of part matching struct empobj */
@@ -86,6 +87,7 @@ struct natstr {
     short ef_type;
     short nat_uid;		/* equals nat_cnum */
     unsigned nat_seqno;
+    unsigned nat_generation;
     time_t nat_timestamp;
     natid nat_cnum;		/* our country number */
     /* end of part matching struct empobj */
diff --git a/include/news.h b/include/news.h
index 24554f096..01a47826b 100644
--- a/include/news.h
+++ b/include/news.h
@@ -49,6 +49,7 @@ struct nwsstr {
     short ef_type;
     short nws_uid;
     unsigned nws_seqno;
+    unsigned nws_generation;
     time_t nws_timestamp;
     /* end of part matching struct empobj */
     natid nws_ano;		/* "actor" country # */
diff --git a/include/nuke.h b/include/nuke.h
index 231f0a045..bb1efd533 100644
--- a/include/nuke.h
+++ b/include/nuke.h
@@ -45,6 +45,7 @@ struct nukstr {
     short ef_type;
     short nuk_uid;
     unsigned nuk_seqno;
+    unsigned nuk_generation;
     time_t nuk_timestamp;	/* Last time this nuke was touched */
     natid nuk_own;
     coord nuk_x, nuk_y;		/* current loc of device */
diff --git a/include/plane.h b/include/plane.h
index 8e37d0d2b..5e809e2df 100644
--- a/include/plane.h
+++ b/include/plane.h
@@ -49,6 +49,7 @@ struct plnstr {
     short ef_type;
     short pln_uid;		/* plane unit id */
     unsigned pln_seqno;
+    unsigned pln_generation;
     time_t pln_timestamp;	/* Last time this plane was touched */
     natid pln_own;		/* owning country */
     coord pln_x;		/* plane x-y */
diff --git a/include/sect.h b/include/sect.h
index 336b71f11..f242b22f6 100644
--- a/include/sect.h
+++ b/include/sect.h
@@ -47,6 +47,7 @@ struct sctstr {
     short ef_type;
     short sct_uid;		/* equals XYOFFSET(sct_x, sct_y) */
     unsigned sct_seqno;
+    unsigned sct_generation;
     time_t sct_timestamp;	/* Last time this sector was written to */
     natid sct_own;		/* owner's country num */
     coord sct_x;		/* x coord of sector */
diff --git a/include/ship.h b/include/ship.h
index 21e749f58..a8762d578 100644
--- a/include/ship.h
+++ b/include/ship.h
@@ -66,6 +66,7 @@ struct shpstr {
     short ef_type;
     short shp_uid;		/* unit id (ship #) */
     unsigned shp_seqno;
+    unsigned shp_generation;
     time_t shp_timestamp;	/* Last time this ship was touched. */
     natid shp_own;		/* owner's country num */
     coord shp_x;		/* x location in abs coords */
diff --git a/include/trade.h b/include/trade.h
index 6fc9919e2..26449ae84 100644
--- a/include/trade.h
+++ b/include/trade.h
@@ -45,6 +45,7 @@ struct trdstr {
     short ef_type;
     short trd_uid;
     unsigned trd_seqno;
+    unsigned trd_generation;
     time_t trd_timestamp;
     natid trd_owner;
     /* end of part matching struct empobj */
diff --git a/include/treaty.h b/include/treaty.h
index 0e164eef5..e70c2f986 100644
--- a/include/treaty.h
+++ b/include/treaty.h
@@ -42,6 +42,7 @@ struct trtstr {
     short ef_type;
     short trt_uid;
     unsigned trt_seqno;
+    unsigned trt_generation;
     time_t trt_timestamp;
     /* end of part matching struct empobj */
     natid trt_cna;		/* proposer */
diff --git a/src/lib/common/file.c b/src/lib/common/file.c
index 908b024b7..16633eee9 100644
--- a/src/lib/common/file.c
+++ b/src/lib/common/file.c
@@ -52,9 +52,12 @@ static int do_read(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 must_be_fresh(struct empfile *, void *);
 static void do_blank(struct empfile *, void *, int, int);
 static int ef_check(int);
 
+static unsigned ef_generation;
+
 /*
  * Open the file-backed table TYPE (EF_SECTOR, ...).
  * HOW are flags to control operation.  Naturally, immutable flags are
@@ -338,6 +341,7 @@ ef_read(int type, int id, void *into)
 	cachep = ep->cache + (id - ep->baseid) * ep->size;
     }
     memcpy(into, cachep, ep->size);
+    ef_mark_fresh(type, into);
 
     if (ep->postread)
 	ep->postread(id, into);
@@ -500,9 +504,11 @@ ef_write(int type, int id, void *from)
 	if (ep->onresize && ep->onresize(type) < 0)
 	    return 0;
     }
-    if (id >= ep->baseid && id < ep->baseid + ep->cids)
+    if (id >= ep->baseid && id < ep->baseid + ep->cids) {
 	cachep = ep->cache + (id - ep->baseid) * ep->size;
-    else
+	if (cachep != from)
+	    must_be_fresh(ep, from);
+    } else
 	cachep = NULL;
     if (ep->prewrite)
 	ep->prewrite(id, cachep, from);
@@ -586,6 +592,35 @@ new_seqno(struct empfile *ep, void *buf)
     elt->seqno = old_seqno + 1;
 }
 
+void
+ef_make_stale(void)
+{
+    ef_generation++;
+}
+
+void
+ef_mark_fresh(int type, void *buf)
+{
+    struct empfile *ep;
+
+    if (ef_check(type) < 0)
+	return;
+    ep = &empfile[type];
+    if (!(ep->flags & EFF_TYPED))
+	return;
+    ((struct emptypedstr *)buf)->generation = ef_generation;
+}
+
+static void
+must_be_fresh(struct empfile *ep, void *buf)
+{
+    struct emptypedstr *elt = buf;
+
+    if (!(ep->flags & EFF_TYPED))
+	return;
+    CANT_HAPPEN(elt->generation != ef_generation);
+}
+
 /*
  * Extend table TYPE by COUNT elements.
  * Any pointers obtained from ef_ptr() become invalid.
@@ -663,6 +698,7 @@ ef_blank(int type, int id, void *buf)
 	elt = buf;
 	elt->seqno = get_seqno(ep, elt->uid);
     }
+    ef_mark_fresh(type, buf);
 }
 
 /*
diff --git a/src/lib/empthread/io.c b/src/lib/empthread/io.c
index 5bbce8d97..3fcf164bf 100644
--- a/src/lib/empthread/io.c
+++ b/src/lib/empthread/io.c
@@ -51,6 +51,7 @@
 #include <unistd.h>
 #include "empio.h"
 #include "empthread.h"
+#include "file.h"
 #include "ioqueue.h"
 #include "misc.h"
 #include "queue.h"
@@ -123,6 +124,9 @@ io_input(struct iop *iop, int waitforinput)
     int cc;
     int res;
 
+    if (waitforinput)
+	ef_make_stale();
+
     /* Not a read IOP */
     if ((iop->flags & IO_READ) == 0) {
 	errno = EBADF;
@@ -183,6 +187,9 @@ io_output(struct iop *iop, int waitforoutput)
     int n;
     int remain;
 
+    if (waitforoutput)
+	ef_make_stale();
+
     /* If there is no output waiting. */
     if (!io_outputwaiting(iop))
 	return 0;
@@ -264,6 +271,9 @@ io_write(struct iop *iop, char *buf, int nbytes, int doWait)
 {
     int len;
 
+    if (doWait)
+	ef_make_stale();
+
     if ((iop->flags & IO_WRITE) == 0)
 	return -1;
     ioq_append(iop->output, buf, nbytes);
@@ -285,6 +295,8 @@ io_output_all(struct iop *iop)
 {
     int n;
 
+    ef_make_stale();
+
     /*
      * Mustn't block a player thread while update is pending, or else
      * a malicous player could delay the update indefinitely
diff --git a/src/lib/empthread/lwp.c b/src/lib/empthread/lwp.c
index 03bf4a64d..66015a78c 100644
--- a/src/lib/empthread/lwp.c
+++ b/src/lib/empthread/lwp.c
@@ -37,6 +37,7 @@
 #include <signal.h>
 #include <time.h>
 #include "empthread.h"
+#include "file.h"
 #include "misc.h"
 
 /* Flags that were passed to empth_init() */
@@ -65,6 +66,7 @@ empth_create(void (*entry)(void *), int size, int flags,
 {
     if (!flags)
 	flags = empth_flags;
+    ef_make_stale();
     return lwpCreate(1, entry, size, flags, name, 0, 0, ud);
 }
 
@@ -89,18 +91,21 @@ empth_set_name(empth_t *thread, char *name)
 void
 empth_exit(void)
 {
+    ef_make_stale();
     lwpExit();
 }
 
 void
 empth_yield(void)
 {
+    ef_make_stale();
     lwpYield();
 }
 
 int
 empth_select(int fd, int flags, struct timeval *timeout)
 {
+    ef_make_stale();
     return lwpSleepFd(fd, flags, timeout);
 }
 
@@ -113,6 +118,7 @@ empth_wakeup(empth_t *a)
 int
 empth_sleep(time_t until)
 {
+    ef_make_stale();
     return lwpSleepUntil(until);
 }
 
@@ -123,6 +129,7 @@ empth_wait_for_signal(void)
     int sig, err;
     time_t now;
 
+    ef_make_stale();
     sigemptyset(&set);
     sigaddset(&set, SIGHUP);
     sigaddset(&set, SIGINT);
@@ -153,12 +160,14 @@ empth_rwlock_destroy(empth_rwlock_t *rwlock)
 void
 empth_rwlock_wrlock(empth_rwlock_t *rwlock)
 {
+    ef_make_stale();
     lwp_rwlock_wrlock(rwlock);
 }
 
 void
 empth_rwlock_rdlock(empth_rwlock_t *rwlock)
 {
+    ef_make_stale();
     lwp_rwlock_rdlock(rwlock);
 }
 
diff --git a/src/lib/subs/check.c b/src/lib/subs/check.c
index f20f96172..6623a3ef5 100644
--- a/src/lib/subs/check.c
+++ b/src/lib/subs/check.c
@@ -52,7 +52,11 @@ obj_changed(struct empobj *obj, size_t sz)
     get_empobj(obj->ef_type, obj->uid, &old);
     memcpy(&tobj, obj, sz);
     old.gen.timestamp = tobj.gen.timestamp = 0;
-    return memcmp(&tobj, &old, sz);
+    old.gen.generation = tobj.gen.generation = 0;
+    if (memcmp(&tobj, &old, sz))
+	return 1;
+    ef_mark_fresh(obj->ef_type, obj);
+    return 0;
 }
 
 int
-- 
2.43.0