Re: [RFC] cbdata NG

On 07/07/2012 09:47 PM, Amos Jeffries wrote:
> On 8/07/2012 12:54 p.m., Alex Rousskov wrote:
>> On 07/07/2012 02:19 PM, Henrik Nordström wrote:
>>> But I remember now that there is one catch. There is some places where
>>> we have needed to use explicit cbdataInternalLock() calls by the owner
>>> of the object to protect from races on cancellation. These need to get
>>> fixed as well.

>> I see three such cases: tunnel.cc, log/ModDaemon.cc, and
>> store_client.cc. Some of those are probably no longer needed because
>> close notifications are now async (cancellation races were not caused by
>> cbdata; they were caused by "reentrant" sync calls). I have not checked
>> the details though.

> If removal is not working a local CbcPointer variable would fix these
> would it not? since the CbcPointer creates a local-scope reference lock
> without the handler needing explicit cbdataInternal*().

No, locking alone does not fix these. The locked object can still be
destructed, screwing up the object owner code higher in the call stack.
Here is the sketch of a problematic "reentrant" sequence:

Without locking:

  Owner::fooCallback()
      conn.close()
          Owner::connCloseCallback()
              delete this; // no new callbacks after this
              // Owner destructor is called
              // Owner memory is deallocated
              return;
      if (this->foo) // coredump as we returned to the 1st cb

With locking:

  Owner::fooCallback()
      this->lock();
      conn.close()
          Owner::connCloseCallback()
              delete this; // no new callbacks after this
              // Owner destructor is called
              // Owner memory is NOT deallocated
              return;
      if (*this->foo) // coredump as we returned to the 1st cb
      this->unlock();

In both cases, the problem is that we must not use object after its
destructor has run because the object may be in inconsistent/invalid
state after the destructor returns. Whether the object memory is
deallocated or not is not really important in this context.

When all callbacks are async, the problem is gone because the
Owner::connCloseCallback() will not be running inside Owner::fooCallback().

HTH,

Alex.
Received on Sun Jul 08 2012 - 15:58:57 MDT