rx: Return success value when cancelling an event

When cancelling an event that holds reference counts, we need
to know whether the attempt to cancel the event was successful
or not, otherwise references can be double put when the
cancellation races with the event firing. Take the follwing

Thread A			Event Thread
=========			============

				... event fired ...
MUTEX_ENTER(&obj->lock);
if (obj->event) {
    rxevent_Cancel(&obj->event);
    obj_put(&obj->refcnt);
}
MUTEX_EXIT(&obj->lock)
				MUTEX_ENTER(&obj->lock);
				if (event == obj->event)
				    rxevent_Put(&obj->event);
				...
				MUTEX_EXIT(&obj->lock);
				obj_put(&obj->refcnt);

Holding obj->lock doesn't control whether the event is fired or
not. Only putting the reference if the event being fired matches
that in obj doesn't help - it just leaks a reference in a different
race.

So, make rxevent_Cancel return true if it did actually cancel the
event, and false if the event has already been fired. This means
that Thread A can become

MUTEX_ENTER(&obj->lock);
if (rxevent_Cancel(&obj->event)
    obj_put(&obj->refcnt);
MUTEX_EXIT(&obj->lock)

In the example above, rxevent_Cancel would return false.

Change-Id: I48e012774c97c9d9588b00687428a32795be2b37
Reviewed-on: http://gerrit.openafs.org/8539
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

diff --git a/src/rx/rx_event.c b/src/rx/rx_event.c
index 0807dc0..99d46b6 100644 (file)
--- a/src/rx/rx_event.c
+++ b/src/rx/rx_event.c
@@ -313,13 +313,21 @@ resetFirst(struct rxevent *ev)
        eventTree.first = NULL;
 }
 
-void
+/*!
+ * Cancel an event
+ *
+ * Cancels the event pointed to by evp. Returns true if the event has
+ * been succesfully cancelled, or false if the event has already fired.
+ */
+
+int
 rxevent_Cancel(struct rxevent **evp)
 {
     struct rxevent *event;
+    int cancelled = 0;
 
     if (!evp || !*evp)
-       return;
+       return 0;
 
     event = *evp;
 
@@ -362,12 +370,15 @@ rxevent_Cancel(struct rxevent **evp)
        }
        event->handled = 1;
        rxevent_put(event); /* Dispose of eventTree reference */
+       cancelled = 1;
     }
 
     MUTEX_EXIT(&eventTree.lock);
 
     *evp = NULL;
     rxevent_put(event); /* Dispose of caller's reference */
+
+    return cancelled;
 }
 
 /* Process all events which have expired. If events remain, then the relative

diff --git a/src/rx/rx_event.h b/src/rx/rx_event.h
index 23c06c9..ae59486 100644 (file)
--- a/src/rx/rx_event.h
+++ b/src/rx/rx_event.h
@@ -30,7 +30,7 @@ extern struct rxevent *rxevent_Post(struct clock *when, struct clock *now,
 /* Remove the indicated event from the event queue.  The event must be
  * pending.  Note that a currently executing event may not cancel itself.
  */
-extern void rxevent_Cancel(struct rxevent **);
+extern int rxevent_Cancel(struct rxevent **);
 
 /* The actions specified for each event that has reached the current clock
  * time will be taken.  The current time returned by GetTime is used

diff --git a/src/rx/rx_prototypes.h b/src/rx/rx_prototypes.h
index 763a95e..1fe2cd8 100644 (file)
--- a/src/rx/rx_prototypes.h
+++ b/src/rx/rx_prototypes.h
@@ -220,7 +220,6 @@ extern struct rxevent *rxevent_Post(struct clock *when, struct clock *now,
 extern void shutdown_rxevent(void);
 extern struct rxepoch *rxepoch_Allocate(struct clock *when);
 extern void rxevent_Init(int nEvents, void (*scheduler) (void));
-extern void rxevent_Cancel(struct rxevent **ev);
 extern int rxevent_RaiseEvents(struct clock *next);