Re: NSDecrementExtraRefCountWasZero

[David Chisnall]

On 30 Nov 2017, at 08:52, Richard Frith-Macdonald 
<richard.frith-macdonald@theengagehub.com> wrote:
> 
> 
> 
>> On 29 Nov 2017, at 12:43, David Chisnall <David.Chisnall@cl.cam.ac.uk> wrote:
>> 
>> Hello the list,
>> 
>> In trying to reduce lock contention for deallocation, I took a bit from the 
>> reference count to indicate that weak references exist to an object (and 
>> therefore avoid any serialisation on deallocation).  Unfortunately, when 
>> Andreas tried testing it, he discovered that this broke 
>> NSDecrementExtraRefCountWasZero (and, in theory, NSIncrementExtraRefCount, 
>> though not in a way that’s likely to be observable).
>> 
>> On OS X, this function is unavailable in ARC mode and comes with a warning 
>> that if any code uses ARC then it may give surprising results.
>> 
>> Looking in GNUstep, it appears that this is called in only a small number of 
>> places:
>> 
>> - NSObject is using it, but probably should just be calling objc_retain() / 
>> objc_release() / objc_autorelease() (the runtime will call these directly 
>> instead of retain / release anyway).  NSProxy, similarly, should simply be 
>> calling the relevant functions.
>> 
>> - A few classes are calling it to avoid deallocation of objects that are in 
>> a table for reuse.It’s also not clear whether this is still a win: we’ll get 
>> more cache contention from refcount manipulations if the objects are small, 
>> such as NSIndexPath (though it is required for correctness in a few cases, 
>> such as NSConnection).  In these cases, if the runtime supports ARC then we 
>> can simply use a strong-to-weak NSHashTable and get rid of most of this 
>> logic.
> 
> That would be good ... at some point it would be great if the core libraries 
> could make use of ARC.

This should be fairly easy now, but only if we’re willing to drop support for 
GCC - it’s very difficult to maintain code that can compile as both ARC and 
non-ARC.

> 
>> I’d like to start making these changes, and expose some new libobjc2 
>> functions that can provide the functionality of 
>> NSDecrementExtraRefCountWasZero (NSIncrementExtraRefCount can be a trivial 
>> wrapper around objc_retain().  Currently, the only way I can see of 
>> implementing NSDecrementExtraRefCountWasZero on top of the ARC APIs that I 
>> can see is to take a weak reference to the object, release it, and then see 
>> if the reference is 0.  
> 
> Presumably the downside of that is fairly severe inefficiency, but since thew 
> functions are rarely used that's probably not a big issue; if it turns out a 
> class using the reference count functions has a major performance impace when 
> used with ARC, I guess that class could be rerwritten.

Thinking a bit more, it’s actually not a good solution, because that will 
deallocate the object, so I will need to add a new runtime hook.  This will 
probably involve some juggling with weak functions.

>> It’s also worth noting that both NSDecrementExtraRefCountWasZero and 
>> NSIncrementExtraRefCount are currently broken (and will cause memory 
>> corruption) if called on blocks (or classes, constant strings, and so on).
> 
> I don't think that's a practical issue since it's always been clear that 
> those functions are provided for classes to manage their own reference 
> counting.   I wouldn't worry about trying to make them work for any/all args.

I’ve been going through the uses in -base and converting them to use weak 
references. I’ve come across what looks like a bug in NSSocketPort.m.  We 
create an NSMapTable with NSObjectMapKeyCallBacks and 
NSNonOwnedPointerMapValueCallBacks.  We then use portNum (a uint16_t) as the 
key.  I don’t understand how this doesn’t crash at run time (that said, I don’t 
know if I’ve ever used anything that uses NSSocketPort).

David