On Wed, 2009-07-29 at 15:28 -0400, Dale Worley wrote:
> This is a revised version of the locking scheme for SipRefreshManager.
>
> Problem: We have a set of objects, and the objects contain timers.
> When a timer fires, the event routine may modify the object containing
> the timer. In addition, methods called by other threads may modify
> the objects, and may create and delete objects. The nastiest part of
> the problem is the race between an externally-called method that wants
> to delete an object and the event routine of a timer within the
> object.
>
> Limitation: We assume that a timer event routine does not affect
> objects other than the one that contains it, and does not delete its
> object.
>
> Proposed solution:
>
> The set is given a lock. There is another lock for access to (all of)
> the objects.
>
> If a thread (but not a timer) possesses a pointer to an object, the
> thread must hold the set lock. If a thread wishes to access an
> object (with the exception of stopping one of the object timers), it
> must also hold the object lock.
>
> If a thread wishes to create or delete an object, it must hold the
> set lock. Since this excludes other threads from possessing a
> pointer to the object, the thread need not hold the object lock if
> there are no running timers in the object.
>
> A timer (that is started) is treated much like a thread. It need not
> hold the set lock as the timer's existence implies that its containing
> object exists. But in order to read or modify the object (especially
> including restarting itself or other timers), it must hold the object
> lock.
>
> In order to prevent deadlocks, if a thread wants to hold both the set
> lock and the object lock, it must seize the set lock first.
>
> Each object contains a boolean member, SuppressTimerEventRoutine.
> This value is normally false. If it is true, then the body of the
> timer event routine for every timer in the object is skipped. A
> consequence of this is that a synchronous stop() applied to a timer
> when it's object SuppressTimerEventRoutine is true will not start any
> timer, and will it hold the object lock only transiently. Reading and
> writing SuppressTimerEventRoutine is protected by the object lock.
>
> Pattern for timer event routine:
>
> seize the object lock
> if not SuppressTimerEventRoutine
> then
> perform the operations
> end if
> release the object lock
>
> Pattern for an externally-called method that does not affect timers:
>
> seize the set lock
> look up the object in question
> seize the object lock
> perform the operations
> release the object lock
> release the set lock
>
> Most externally-called methods will cause state changes in the object
> for which timers should be stopped and started. In that case, a more
> elaborate pattern is needed:
>
> seize the set lock
> look up the object in question
> seize the object lock
> set SuppressTimerFire
> release the object lock
> # After this point, any timer event routines for the object
> # that are executed will not start any timers.
> stop timer(s) synchronously
> # At this point all the object's timers are stopped and
> # there are no queued event routine firings.
> seize the object lock
> clear SuppressTimerFire
> perform the operations (which may include starting timers)
> release the object lock
> release the set lock
>
> Pattern for an externally-called method to delete an object:
>
> seize the set lock
> look up the object in question
> seize the object lock
> set SuppressTimerFire
> release the object lock
> # After this point, any timer event routines for the object
> # that are executed will not start any timers.
> stop timer(s) synchronously
> # At this point all the object's timers are stopped and
> # there are no queued event routine firings.
> remove the object from the set
> delete the object
> release the set lock
>
> Pattern for an externally-called method to create an object:
>
> seize the set lock
> create the object
> seize the object lock
> clear SuppressTimerFire
> perform the operations (which may include starting timers)
> release the object lock
> release the set lock
>
> In SipRefreshManager, timer event routines or externally-called
> methods may want to send SIP messages. Since SipUserAgent::send() can
> take a long time, the routine must pass responsibility for doing this
> to the SipRefreshManager's thread, by queueing a message for
> SipRefreshManager::handleMessage to act on. (It cannot be done by a
> timer event routine, even after it has released the object lock,
> because a method (that holds the set lock) can be waiting for the
> timer event routine to finish.) This introduces new race conditions,
> in that a message may be sent after the object state changes in a way
> that renders the message obsolete, but SIP already allows for
> duplicated or delayed messages, so no harm can come from this.
>
> Similarly, any call-back routines to be called must be called from
> SipRefreshManager::handleMessage. This also creates race conditions,
> but callback routines are not expected to be completely synchronous.
This looks good to me. It may be that you can reduce the time that the
set lock is held by modifying one rule:
Pattern for an externally-called method that does not affect
timers:
seize the set lock
look up the object in question
seize the object lock
release the set lock
perform the operations
release the object lock
whether or not this is worth the extra complexity depends on whether or
not these are common and how long they take.
_______________________________________________
sipx-dev mailing list [email protected]
List Archive: http://list.sipfoundry.org/archive/sipx-dev
Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
sipXecs IP PBX -- http://www.sipfoundry.org/
