commit 048a2e91193b686346ed49f1750a50a7278333ef
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Sat Oct 7 22:42:38 2017 -0500

    adopt event-t to new semantics
    
    Change-Id: I374b51bf182f6cf06f96098ecdb962902c95de52

diff --git a/tests/rx/event-t.c b/tests/rx/event-t.c
index b9ba7c85c..71db9ecf5 100644
--- a/tests/rx/event-t.c
+++ b/tests/rx/event-t.c
@@ -45,7 +45,8 @@ eventSub(struct rxevent *event, void *arg, void *arg1, int arg2)
     struct testEvent *evrecord = arg;
 
     pthread_mutex_lock(&eventListMutex);
-    rxevent_Put(&evrecord->event);
+    if (evrecord->event != NULL)
+	rxevent_Put(&evrecord->event);
     evrecord->event = NULL;
     evrecord->fired = 1;
     pthread_mutex_unlock(&eventListMutex);
@@ -95,7 +96,7 @@ eventHandler(void *dummy) {
 int
 main(void)
 {
-    int when, counter, fail, fired, cancelled;
+    int when, counter, fail, fired, cancelled, victim;
     struct clock now, eventTime;
     struct rxevent *event;
     pthread_t handler;
@@ -144,15 +145,18 @@ main(void)
 	}
 
 	/* A 25% chance that we will cancel a random event */
+	event = NULL;
 	if (random() % 4 == 0) {
-	    int victim = random() % counter;
+	    victim = random() % counter;
 
 	    if (events[victim].event != NULL) {
-		rxevent_Cancel(&events[victim].event);
-		events[victim].cancelled = 1;
+		event = events[victim].event;
+		events[victim].event = NULL;
 	    }
 	}
 	pthread_mutex_unlock(&eventListMutex);
+	if (rxevent_Cancel(&event))
+	    events[victim].cancelled = 1;
     }
 
     ok(1, "Added %d events", NUMEVENTS);

commit a6056be8b4616d78b10efe710a3a439f17f2a5a9
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Sat Oct 7 22:33:57 2017 -0500

    Update rxevent semantics to avoid cancel/handler race
    
    Change-Id: I63227efc340b13c99d55fa026bdc6d0f30a6513f

diff --git a/src/rx/rx_event.c b/src/rx/rx_event.c
index 6638e5be6..04669cbf8 100644
--- a/src/rx/rx_event.c
+++ b/src/rx/rx_event.c
@@ -64,6 +64,7 @@ struct rxevent {
     struct clock eventTime;
     struct rxevent *next;
     rx_atomic_t refcnt;
+    /* handled==1 while handler runs; handled==2 when fully done */
     int handled;
     void (*func)(struct rxevent *, void *, void *, int);
     void *arg;
@@ -99,6 +100,8 @@ static struct {
 
 static int allocUnit = 10;
 
+static afs_kcondvar_t cancel_race_cv;
+
 static struct rxevent *
 rxevent_alloc(void) {
      struct rxevent *evlist;
@@ -239,6 +242,8 @@ rxevent_Init(int nEvents, void (*scheduler)(void))
     opr_queue_Init(&freeEvents.list);
     freeEvents.mallocs = NULL;
 
+    CV_INIT(&cancel_race_cv, "rx event race", CV_DEFAULT, 0);
+
     if (nEvents)
 	allocUnit = nEvents;
 
@@ -320,6 +325,17 @@ resetFirst(struct rxevent *ev)
  *
  * Cancels the event pointed to by evp. Returns true if the event has
  * been succesfully cancelled, or false if the event has already fired.
+ *
+ * When returning false, the non-cancelled event is guaranteed to have
+ * completed running the event handler function before rxevent_Cancel()
+ * returns.
+ *
+ * Must not be called while holding any locks that might be taken by the
+ * handler function for the indicated event, and |*evp| must not be referenced
+ * by any other thread.  In practice, this means that the caller must (under the
+ * appropriate lock) remove the pointer from a locked data structure and transfer
+ * the reference to a local variable, unlock the lock, and then call
+ * rxevent_Cancel() with the local variable's address.
  */
 
 int
@@ -335,7 +351,7 @@ rxevent_Cancel(struct rxevent **evp)
 
     MUTEX_ENTER(&eventTree.lock);
 
-    if (!event->handled) {
+    if (event->handled == 0) {
 	/* We're a node on the red/black tree. If our list is non-empty,
 	 * then swap the first element in the list in in our place,
 	 * promoting it to the list head */
@@ -370,10 +386,13 @@ rxevent_Cancel(struct rxevent **evp)
 		opr_rbtree_remove(&eventTree.head, &event->node);
 	    }
 	}
-	event->handled = 1;
+	event->handled = 2;
 	rxevent_put(event); /* Dispose of eventTree reference */
 	cancelled = 1;
+    } else if (event->handled == 1) {
+	CV_WAIT(&cancel_race_cv, &eventTree.lock);
     }
+    /* else event->handled == 2; everything's done */
 
     MUTEX_EXIT(&eventTree.lock);
 
@@ -427,9 +446,11 @@ rxevent_RaiseEvents(struct clock *wait)
 
         /* Fire the event, then free the structure */
 	event->func(event, event->arg, event->arg1, event->arg2);
-	rxevent_put(event);
 
 	MUTEX_ENTER(&eventTree.lock);
+	event->handled = 2;
+	CV_BROADCAST(&cancel_race_cv);
+	rxevent_put(event);
     }
 
     /* Figure out when we next need to be scheduled */
@@ -456,6 +477,8 @@ shutdown_rxevent(void)
     }
     MUTEX_DESTROY(&eventTree.lock);
 
+    CV_DESTROY(&cancel_race_cv);
+
 #if !defined(AFS_AIX32_ENV) || !defined(KERNEL)
     MUTEX_DESTROY(&freeEvents.lock);
     mrec = freeEvents.mallocs;