Re: RFA: NSAutoreleasePool ivar cleanup

[Richard Frith-Macdonald]

On 24 Mar 2011, at 09:24, David Ayers wrote:

> Am Donnerstag, den 24.03.2011, 06:30 +0000 schrieb Richard
> Frith-Macdonald: 
>> On 24 Mar 2011, at 06:00, David Ayers wrote:
>> 
>>> Hi folks,
>>> 
>>> I was chasing a sporadic (very seldom) crash which we noticed on
>>> production instances.  The version of the libraries we are using is
>>> ancient.  But the issue may still exist.
>>> 
>>> in NSAutoreleasePool -emptyPool there is this comment:
>>>     /* If there are NSAutoreleasePool below us in the stack of
>>>        NSAutoreleasePools, then deallocate them also.  The (only) way
>>> we
>>>        could get in this situation (in correctly written programs,
>>> that
>>>        don't release NSAutoreleasePools in weird ways), is if an
>>>        exception threw us up the stack. */
>>>     while (_child != nil)
>>>       {
>>>         [_child dealloc];
>>>       }
>>> 
>>> well... we may have an incorrectly written program here and we may not
>>> catch alle exceptions properly but I currently cannot tell for sure.
>>> But whether correct or not, I believe the _child ivar should be cleared
>>> after the dealloc since release pools get recycled.
>> 
>> If you look at the implementation of -dealloc, you will see that in it
>> the child removes itsself from its parent (by clearing
>> parent->_child).
>> You can be sure that _child is set to nil ... if it didn't, the while
>> loop would never terminate.
>> Your patch, setting _child to nil after deallocating it, ought to have
>> no effect (other than wasting a tiny amount of cpu time) unless you
>> have some problem elsewhere which is corrupting the _child ivar during
>> the call to dealloc (after the point where it is set to nil). As far
>> as I can see the only thing going on there is pushing the deallocated
>> pool to the cache ... perhaps a problem in the cache? More likely I
>> would have thought, would be memory corruption caused by another
>> thread.
> 
> The patch set's the _child ivar of the receiver of emptyPool to nil.
> The dealloc implementation set's the _child ivar of the _child to nil.

The -dealloc implementation also sets the _child ivar of the receiver of 
-emptyPool to nil.

> I think the _child ivar of the receiver of emtpyPool will retain the
> reference to the cached but invalidated pool.

No, because [_child dealloc] will have set _child to nil.

Consider two pools, R and C where C is the child of R.

R->_child == C
C->_parent = R

If you call [R emptyPool]
that calls [_child dealloc] (ie [C dealloc])
and [C dealloc] sets C->_parent->_child to nil and then sets C->_parent t nil 
as well
so when you get back into [R emptyPool], R->_child (which was 
C->_parent->_child) has been set to nil and the while loop terminates.