[Openais] Re: just looked at some of the openais code, have some comments

Steven Dake sdake at mvista.com
Tue Jun 29 15:00:12 PDT 2004


On Tue, 2004-06-29 at 13:50, Chris Friesen wrote:
> Steven Dake wrote:
> > Chris,
> > 
> > Find attached a copy of a patch that uses reference counting for
> > tracking users of a handle.
> 
> I don't actually see the definition of saRefCount in the patch.  Not important 
> for the comments below.
> 
> > Some design notes:
> > 1. saHandleCreate set the reference count to 1 (this is called by the
> > initialize functions of each API)
> 
> Actually, I would suggest that a create should set the refcount to 1 if 
> instanceOut is null, or 2 if instanceOut is non-null.  Creation implies adding 
> it to the database, which is one reference.  Passing back a pointer to the 
> instance to the caller is another reference.
> 
> Also, I just noticed that there is actually a bug in saHandleCreate() in that it 
> doesn't handle instanceOut being null.
> 
> > 2. saHandleGet will not allow a handle to be taken if the reference
> > count is 0 even if it is valid in the database
> 
> That check shouldn't be required. Finalize() should result in the instance being 
> removed from the database, and decrementing the refcount.  Refcount hitting zero 
> should immediately result in free'ing the instance.
> 
> Here's how it could work:
> 
> saHandlePut() should take an instance variable.  That way, the instance can 
> exist without having to be in the database.
> 
> The call sequence then could look like this:
> 
> Thread 1 calls saHandleCreate() with a null instanceOut. (instance refcount is 
> now 1).
> 
> Thread 1 calls saHandleGet(), and gets an instance (refcount is now 2).
> 
> Thread 2 calls saHandleRemove(), which would result in the instance being 
> removed from the database (but not free()'d).  Since the database no longer has 
> a reference to the instance, saHandleRemove() calls saHandlePut() which 
> decrements the refcount. (refcount now 1).
> 
> Thread 3 calls saHandleGet() and fails, since instance no longer in database.
> 
> Thread 1 calls saHandlePut() (passing it the instance that it was using) since 
> its done with the instance.  saHandlePut() decrements the refcount, which 
> becomes zero, and then free()'s the instance.
> 
> And now that I think about it, saHandleGet/Put should really be 
> saInstanceGet/Put, since you're passing in a handle and getting back an instance.
> 
> > 3. The finalize does a get to get the instance data, a put to release
> > the reference to the instance data, a put to release the reference from
> > the initialize call (so if there are no users, it is now 0), and finally
> > a wait for refcount to go to zero before deleting.
> 
> Finalize shouldn't have to wait for anything if the refcounts are done as above. 
>   The finalize function would call saHandleGet(), then call saHandleRemove() to 
> remove the entity from the database.  That in turn calls saHandlePut(), which 
> will decrement the refcount.  The finalize function then takes the instance 
> mutex (since other threads could still have references to the instance), does 
> the instance cleanup (shutdown, close, sets finalize=1), then drops the mutex 
> and calls saHandlePut().  If nobody else has references to the instance, then 
> the refcount drops to zero at that point and saHandlePut() will free up the 
> instance.
> 
> No waiting required--whoever drops the refcount to zero ends up doing the free().
> 
> 

This sounds like a reasonable approach.  Your suggestion made me
consider that shutdown and close must be done when the ref count drops
to zero, not before.  One possibility is to register a "refcount goes to
zero" destructor within saHandleDatabase.  This function would be called
by saHandlePut (saHandleInstancePut) when refcount drops to zero and
then do the shutdown and close operations.  Then the saHandlePut
function could free the instance data.

The only problems are the Dispatch functions.  Right now, openais uses
poll to detect the socket has disappeared (because of shutdown), so the
Dispatch knows to exit from the poll.  If shutdown and close must wait
to be executed until ref count drops to zero, the Finalize shutdown()
syscall is never going to be called.  Currently openais has a mechanism
to activate the poll loop which Finalize could execute to have Dispatch
check the finalize bit.  This could be used, or do you have other
suggestions?

Regards
-steve

> Comments?
> 
> Chris
> 
> 
> 




More information about the Openais mailing list