On Fri, 2010-01-08 at 10:36 -0500, Dale Worley wrote:
> On Thu, 2010-01-07 at 16:46 -0500, Scott Lawrence wrote:
> > On Thu, 2010-01-07 at 15:50 -0500, Dale Worley wrote:
> > > More exactly, the problem is that whether and where the lock is held is
> > > not determined by the static structure of the code. That is what OsLock
> > > was designed to allow, and OsLock::release() violates that. Within that
> > > context, I can replace ::release with OsUnLock with only a slight loss
> > > of efficiency, and have the lock structure be statically determined.
> > > With that structure, gotchas are still possible, but much less so than
> > > with manual locking.
> >
> > Sorry - I'm not clear on what change that implies... could you put in a
> > code snippet like the one you started with to illustrate?
>
> Here's an example (with some extraneous code deleted):
>
> OsStatus SipSubscribeClient::handleRestartEvent(const UtlString& handle)
> {
> OsLock lock(mSemaphore);
>
> // Get the SubscriptionGroupState.
> // Due to race conditions, it may no longer exist.
> SubscriptionGroupState* groupState = getGroupStateByOriginalHandle(handle);
> if (groupState)
> {
> // Test whether restart is set.
> // It may be false because the subscription group is being ended.
> if (groupState->mRestart)
> {
> groupState->setRestartTimer();
>
> OsUnLock unlock(lock);
> groupState = NULL; // Remember that groupState is no longer
> reliable,
> // because it points to a structure protected by
> // mSemaphore.
>
> reestablish(handle);
> }
> }
> }
>
> The problem to be solved is that reestablish() needs to be called with
> mSemaphore unlocked. OsUnLock causes mSemaphore to be released within
> its scope.
I see no advantage at all in the above over using the existing acquire
and release interfaces on the underlying semaphores, and a (to my mind)
considerable disadvantage - that the scope of the OsLock is no longer
exactly the scope of the object.
My perception of the value of the OsLock object wrapper is two-fold: it
is easy to use for the simple cases (you don't need to worry about
forgetting to add the release call because the scope will do it), but
also that it makes a nice programmer friendly semantic association
between the block scope and the lock scope. When I do:
{
... code-a ...
{
OsLock critical(mutex);
... code-b ...
}
... code-c ...
}
the inner set of {} become a nice syntactic part of the lock. I know
unambiguously that regardless of how complex it becomes, code-b is all
in the critical region and protected, and neither code-a nor code-c can
be. What you're suggesting preserves the latter, but not the former.
On the other hand, if I see this code:
{
... code-a ...
mutex.acquire();
... code-b ...
mutex.release();
... code-c ...
}
(whether or not there are inner braces), then I know that I have to be
careful everywhere in code-b to keep track of the lock state, because
there could be a release() in there somewhere?
_______________________________________________
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/
