From f40199327a8753d8350b5fefedc2bb556b8b00a6 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Thu, 22 Nov 2012 08:41:51 +0000 Subject: [PATCH] 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 Reviewed-by: Derrick Brashear Reviewed-by: Jeffrey Altman --- src/rx/rx_event.c | 15 +++++++++++++-- src/rx/rx_event.h | 2 +- src/rx/rx_prototypes.h | 1 - 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/rx/rx_event.c b/src/rx/rx_event.c index 0807dc0f8..99d46b6f8 100644 --- 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 23c06c9a6..ae59486ac 100644 --- 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 763a95e9d..1fe2cd82c 100644 --- 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); -- 2.39.5