Re: Another multithreading bug (I think)

[Richard Frith-Macdonald]

On 7 Sep 2006, at 12:23, Richard Frith-Macdonald wrote:
> So ... I think we have to assume that the interpretation of this  
> is ...
> If a release causes the extra reference count to drop to zero then  
> we assume that no other thread can access the object (if another  
> thread could access it, it should have retained it, in which case  
> the extra reference count could not have dropped to zero).
>
> The implication of this seems to be that if we are going to store  
> an object (like a connection) in any sort of collection where it's  
> not retained and should remove itsself upon deallocation, we need  
> to implement custom retain/release methods to manage thread- 
> safety.  So there may well be similar thread safety problems with  
> other classes.
>
> As for the NSConnection class, it seems to me that the simplest  
> solution now is to make nsconnection_table_gate a recursive lock  
> and use it to protect both the retain and release methods (and make  
> very sure that there really are no important side effects when  
> manipulating the table).

I've committed a change which moves the call to -gcFinalise into - 
dealloc and replaces the implementation of -release with:

{
  M_LOCK(connection_table_gate);
  if (NSDecrementExtraRefCountWasZero(self))
    {
      NSHashRemove(connection_table, self);
      M_UNLOCK(connection_table_gate);
      [self dealloc];
    }
  else
    {
      M_UNLOCK(connection_table_gate);
    }
}

Can you see any problems with this?
I don't think -retain needs to lock the gate, and as long as we are  
only locking in the -release method as above, I think we can avoid  
using a recursive lock (as I don't think we call -release inside any  
region where we already lock the gate), but I may have missed something.