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

[Steven Dake]

Chris,

Take a look at this patch

I think it implements most of what you mentioend regarding ref counting
with the destructor idea.

Thanks
-steve


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().
> 
> 
> Comments?
> 
> Chris
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openais-refcount-2.patch
Type: text/x-patch
Size: 76851 bytes
Desc: not available
Url : http://lists.linux-foundation.org/pipermail/openais/attachments/20040629/e4ac224b/openais-refcount-2-0001.bin