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 0807dc0f8485e7ad015b5443660e5edb5b6b4b27..99d46b6f8884bc57a54bc1c423179b7b23516f69 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 23c06c9a68dbb969ec6fb122f7b0f176f1a93587..ae59486ac1e8683b736cedcb36493e362d89e05f 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 763a95e9dbc2b541d56423ff0807c158c10fb7c7..1fe2cd82cac3a5fb3e743d1da1ba7041342cfd14 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);