bugGNUstep - Bugs: bug #41111, C++ destructors not called...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #41111: C++ destructors not called correctly in finalize breaking objc_setAssociatedObject

Submitted by:  Doug Warren <doug_w>
Submitted on:  Sun 05 Jan 2014 06:30:36 PM UTC  
 
Category: Base/FoundationSeverity: 3 - Normal
Item Group: BugStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: In Test

Tue 07 Jan 2014 10:46:54 AM UTC, comment #3:

Hmm … after spending a long time looking at this (and into the runtime code a little), it seems the report is rather misleading.

Firstly, the test case is broken to always fail because it depends on the associated object being deallocated, and that object is in an autorelease pool which won't be deallocated until after the test takes place.
i fixed that.

Secondly, the two code snippets do behave identically on my system at least; the suggested code fix doesn't work.

It seems the actual problem is in the logic of the check for small objects earlier … which was causing the deallocation to start with the wrong class. I reversed the logic of the test in svn trunk, and I think that fixes the issue.

Richard Frith-Macdonald <CaS>
Project Member
Mon 06 Jan 2014 11:44:26 AM UTC, comment #2:

Thanks for the testcase … I added it.
But, please could you clarify if I've misunderstood or if this is a libobjc bug rather than a base bug?

Richard Frith-Macdonald <CaS>
Project Member
Sun 05 Jan 2014 07:32:37 PM UTC, comment #1:

I'm confused.
As far as I can see the two code snippets should have identical effects.
Both step up the class hierarchy, and both call each destructor implementation found.
The only difference seems to be that one stops traversing the hierarchy after the last destructor is called, and the other continues traversing the hierarchy up to the root class.
However, the destructorClass variable is not used … after this, so whether its the first superclass without a destructor or the root class doesn't matter.

Unless … perhaps this is illustrating some bug in class_respondsToSelector() ?
Are you saying that this is returning NO for some cases when it should return YES?
My understanding is that this function should return YES if instances of a class respond to a selector (ie if the method is implemented by the class or by any of its superclasses).

Richard Frith-Macdonald <CaS>
Project Member
Sun 05 Jan 2014 06:30:36 PM UTC, original submission:

NSObject -finalize method does not call C++ destructors correctly in every case. This breaks libobjc2's objc_setAssociatedObject retain policy. I've attached a GNUStep style test that fails to demonstrate this.

The problem looks like it was this optimization:
/* C++ destructors must be called in the opposite order to their
* creators, so start at the leaf class and then go up the tree until we
* get to the root class. As a small optimisation, we don't bother
* visiting any classes that don't have an implementation of this method
* (including one inherited from a superclass).
*
* Care must be taken not to call inherited .cxx_destruct methods.
*/
while (class_respondsToSelector(destructorClass, cxx_destruct))
{
IMP newDestructor;

newDestructor
= class_getMethodImplementation(destructorClass, cxx_destruct);
destructorClass = class_getSuperclass(destructorClass);

if (newDestructor != destructor)
{
newDestructor(self, cxx_destruct);
destructor = newDestructor;
}
}

In my local tree I've replaced it with:
while (destructorClass)
{
if (class_respondsToSelector(destructorClass, cxx_destruct))
{
IMP newDestructor = class_getMethodImplementation(destructorClass, cxx_destruct);
destructorClass = class_getSuperclass(destructorClass);

if (newDestructor != destructor)
{
newDestructor(self, cxx_destruct);
destructor = newDestructor;
}
}
else
{
destructorClass = class_getSuperclass(destructorClass);
}
}

And I've been using it for about a month now with no ill effect.

Doug Warren <doug_w>

 

Attached Files
file #30195:  associated.m added by doug_w (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by CaS (Posted a comment)
  • -unavailable- added by doug_w (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 07 Jan 2014 10:46:54 AM UTCCaSStatusNeed Info=>Fixed
      Open/ClosedOpen=>In Test
    Mon 06 Jan 2014 11:44:26 AM UTCCaSStatusNone=>Need Info
    Sun 05 Jan 2014 06:30:36 PM UTCdoug_wAttached File-=>Added associated.m, #30195

    Back to the top


    Powered by Savane 3.1-cleanup1