The new version still looks more complex than the equivalent (correct) code in libobjc2.
David On 16 Dec 2013, at 23:49, Doug Warren <[email protected]> wrote: > 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; > } > } > > I 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 it works for my test cases though I haven't checked if there was a > performance penalty or not. > > > On Mon, Dec 16, 2013 at 11:30 AM, David Chisnall <[email protected]> wrote: > This sounds like a bug in GNUstep base's object deallocation then. If > .cxx_dealloc isn't called then ObjC++ won't work correctly either. > > David > > On 16 Dec 2013, at 19:24, Doug Warren <[email protected]> wrote: > > > Followup to this, > > > > cleanupReferenceList() is never being called, I see the hidden class > > created and the .cxx_destruct registered but it isn't invoked. The problem > > is that I don't see how object_dispose() get's called. In the libobjc2 > > Test it's called in dealloc which would explain why the libobjc2 Test > > works, but I don't see it invoked in gnustep-base anywhere. Or for that > > matter anywhere in anyone's trunk other than libobjc2's Test.h and > > RuntimeTest.m. > > > > > > On Fri, Dec 13, 2013 at 1:30 PM, Doug Warren <[email protected]> wrote: > > Works without objc_setAssociatedObject: > > E/Z2GameLog(14278): STDOUT: [----------] 1 test from > > objc_setAssociatedObject > > E/Z2GameLog(14278): STDOUT: [ RUN ] > > objc_setAssociatedObject.AssociatedObjectsAreReleased > > E/Z2GameLog(14278): STDOUT: [ OK ] > > objc_setAssociatedObject.AssociatedObjectsAreReleased (1 ms) > > E/Z2GameLog(14278): STDOUT: [----------] 1 test from > > objc_setAssociatedObject (1 ms total) > > > > Code change: > > @autoreleasepool { > > associatedObjectTestAllocatedObject* object = > > [associatedObjectTestAllocatedObject new]; > > > > associatedObjectTestAssociatedObject *info = > > [[associatedObjectTestAssociatedObject new] autorelease]; > > // silence -Wunused-variable > > [info description]; > > // objc_setAssociatedObject(object, &objc_setAssociatedObjectKey, > > info, OBJC_ASSOCIATION_RETAIN); > > > > [object release]; > > } > > > > The reason why I had suggested that was your original post that NSString > > wouldn't be an appropriate target, got me thinking that perhaps there was > > an optimization at some point made in the runtime to NSObject that would > > similarly cause it to not work. > > > > > > On Fri, Dec 13, 2013 at 11:43 AM, David Chisnall <[email protected]> wrote: > > This test case looks like it should pass irrespective of whether the > > runtime does the right thing. Since you're using > > associatedObjectTestAssociatedObject for both objects, deallocating the > > original autoreleased one ought to set associatedObjectDeallocCalled. Can > > you confirm whether this test fails for you on GNUstep with the > > objc_setAssociatedObject() call removed? > > > > David > > > > On 13 Dec 2013, at 18:27, Doug Warren <[email protected]> wrote: > > > > > I saw the test that you checked in to svn and modified my code to match > > > it: > > > > > > static BOOL associatedObjectDeallocCalled = NO; > > > static const char* objc_setAssociatedObjectKey = > > > "objc_setAssociatedObjectKey"; > > > > > > @interface associatedObjectTestAssociatedObject : NSObject > > > @end > > > > > > @implementation associatedObjectTestAssociatedObject > > > > > > -(void) dealloc > > > { > > > associatedObjectDeallocCalled = YES; > > > [super dealloc]; > > > } > > > > > > @end > > > > > > @interface associatedObjectTestAllocatedObject : NSObject > > > @end > > > > > > @implementation associatedObjectTestAllocatedObject > > > @end > > > > > > TEST(objc_setAssociatedObject, AssociatedObjectsAreReleased) > > > { > > > #if !Z2_APPLE > > > GSDebugAllocationActive(YES); > > > GSDebugAllocationList(YES); > > > #endif > > > > > > @autoreleasepool { > > > associatedObjectTestAllocatedObject* object = > > > [associatedObjectTestAllocatedObject new]; > > > > > > associatedObjectTestAssociatedObject *info = > > > [[associatedObjectTestAssociatedObject new] autorelease]; > > > objc_setAssociatedObject(object, &objc_setAssociatedObjectKey, > > > info, OBJC_ASSOCIATION_RETAIN); > > > > > > [object release]; > > > } > > > #if !Z2_APPLE > > > EXPECT_STREQ(GSDebugAllocationList(YES), "1\tNSDataMalloc\n"); > > > #endif > > > > > > ASSERT_TRUE(associatedObjectDeallocCalled); > > > } > > > > > > > > > On the Apple runtime it passes still: > > > [----------] 1 test from objc_setAssociatedObject > > > [ RUN ] objc_setAssociatedObject.AssociatedObjectsAreReleased > > > [ OK ] objc_setAssociatedObject.AssociatedObjectsAreReleased (0 ms) > > > [----------] 1 test from objc_setAssociatedObject (0 ms total) > > > > > > On Gnustep/objc2: > > > E/Z2GameLog(12157): STDOUT: [----------] 1 test from > > > objc_setAssociatedObject > > > E/Z2GameLog(12157): STDOUT: [ RUN ] > > > objc_setAssociatedObject.AssociatedObjectsAreReleased > > > E/Z2GameLog(12157): STDOUT: > > > /data/testnations.git/jujulib/Tests/objc-utility/objc_setAssociatedObjectTest.h:44: > > > Failure > > > E/Z2GameLog(12157): STDOUT: Value of: "1\tNSDataMalloc\n" > > > E/Z2GameLog(12157): STDOUT: Actual: "1 NSDataMalloc > > > E/Z2GameLog(12157): STDOUT: " > > > E/Z2GameLog(12157): STDOUT: Expected: GSDebugAllocationList(((BOOL)1)) > > > E/Z2GameLog(12157): STDOUT: Which is: "1 NSDataMalloc > > > E/Z2GameLog(12157): STDOUT: 1 associatedObjectTestAssociatedObject > > > E/Z2GameLog(12157): STDOUT: " > > > E/Z2GameLog(12157): STDOUT: > > > /data/testnations.git/jujulib/Tests/objc-utility/objc_setAssociatedObjectTest.h:47: > > > Failure > > > E/Z2GameLog(12157): STDOUT: Value of: innerObjectDeallocCalled > > > E/Z2GameLog(12157): STDOUT: Actual: false > > > E/Z2GameLog(12157): STDOUT: Expected: true > > > E/Z2GameLog(12157): STDOUT: [ FAILED ] > > > objc_setAssociatedObject.AssociatedObjectsAreReleased (1 ms) > > > E/Z2GameLog(12157): STDOUT: [----------] 1 test from > > > objc_setAssociatedObject (2 ms total) > > > > > > My runtime environment unfortunately isn't in a position to run the > > > libobjc tests directly. Do you have any suggestions as to what I should > > > be looking for? Perhaps there's something in the GNUStep Foundation > > > NSObject that's causing it to fail but works with the libobjc Test object? > > > > > > > > > On Fri, Dec 13, 2013 at 9:01 AM, Doug Warren <[email protected]> > > > wrote: > > > Thanks David, > > > > > > I'll investigate more on our end. I actually had changed that from > > > NSString to NSObject just to be sure that it wasn't related to that. > > > (Why in the above stupid example it was cast to NSObject though Alloced > > > as NSString, I had intended it to just be a double dummy NSObject.) The > > > actual case where we ran into this was more complex and I feel there's > > > still an issue but I'll go back and confirm it. > > > > > > I was looking at another issue that we had with NSAutoreleasePool and > > > trying to isolate it to a test case. If I get that I'll construct it as > > > a standalone program as well. > > > > > > > > > On Fri, Dec 13, 2013 at 6:44 AM, David Chisnall > > > <[email protected]> wrote: > > > Ah, sorry, I was reading your example the other way around. This seems > > > to be because [[NSString alloc] init] returns a singleton on GNUstep. I > > > don't think this is necessarily a bug, as there's nothing in the contract > > > for NSString that says that it needs to return a new instance for > > > different immutable strings. > > > > > > In general, using associated objects on immutable objects is a bad idea, > > > because it breaks the assumption that you can use equal instances > > > interchangeably. > > > > > > David > > > > > > On 13 Dec 2013, at 14:03, David Chisnall <[email protected]> > > > wrote: > > > > > > > I've now added a test for this to the libobjc2 test suite, but it > > > > passes and so does not appear to be the cause. Your issue appears to > > > > be the dictionary being over-retained. I will investigate further. > > > > > > > > David > > > > > > > > P.S. When sending test cases, it helps to send them in the format of a > > > > test suite for the thing you are testing, or as stand-alone programs. > > > > I'd have got to this a lot sooner if I didn't have to hack on the test > > > > to make it compile. > > > > > > > > On 19 Nov 2013, at 01:33, Doug Warren <[email protected]> wrote: > > > > > > > >> Hi Guys, > > > >> > > > >> Was tracking a memory leak in an existing app and tracked it down to > > > >> objc_setassociatedobject not working as expected from the code being > > > >> developed for the Apple Runtime. The docs around > > > >> objc_setassociatedobject seem to imply it follows the Apple Runtime > > > >> but this simple gtest will pass on Apple but fail on libobjc/GNUStep > > > >> Base: > > > >> > > > >> static BOOL deallocCalled = NO; > > > >> static const char* objc_setAssociatedObjectKey = > > > >> "objc_setAssociatedObjectKey"; > > > >> > > > >> @interface NSMutableDictionary(setAssociatedObjectTest) > > > >> @end > > > >> > > > >> @implementation NSMutableDictionary(setAssociatedObjectTest) > > > >> > > > >> -(void) dealloc > > > >> { > > > >> deallocCalled = YES; > > > >> [super dealloc]; > > > >> } > > > >> > > > >> @end > > > >> > > > >> TEST(objc_setAssociatedObject, AssociatedObjectsAreReleased) > > > >> { > > > >> @autoreleasepool { > > > >> NSObject* object = [[NSString alloc] init]; > > > >> > > > >> NSMutableDictionary *info = [NSMutableDictionary > > > >> dictionaryWithCapacity:1]; > > > >> objc_setAssociatedObject(object, &objc_setAssociatedObjectKey, > > > >> info, OBJC_ASSOCIATION_RETAIN); > > > >> > > > >> [object release]; > > > >> } > > > >> > > > >> ASSERT_TRUE(deallocCalled); > > > >> } > > > >> > > > >> Adding calls to GSDebugAllocationList before/after the autorelease > > > >> pool: > > > >> 1 NSDataMalloc > > > >> 1 GSMutableDictionary > > > >> > > > >> Shows that the dictionary leaks. > > > >> > > > >> Any thoughts? > > > >> _______________________________________________ > > > >> Gnustep-dev mailing list > > > >> [email protected] > > > >> https://lists.gnu.org/mailman/listinfo/gnustep-dev > > > > > > > > > > > > _______________________________________________ > > > > Gnustep-dev mailing list > > > > [email protected] > > > > https://lists.gnu.org/mailman/listinfo/gnustep-dev > > > > > > > > > > > > > > > > > > > -- Sent from my brain > > > > > > > > _______________________________________________ > > Gnustep-dev mailing list > > [email protected] > > https://lists.gnu.org/mailman/listinfo/gnustep-dev > > -- > This email complies with ISO 3103 > > > _______________________________________________ > Gnustep-dev mailing list > [email protected] > https://lists.gnu.org/mailman/listinfo/gnustep-dev -- Sent from my IBM 1620 _______________________________________________ Gnustep-dev mailing list [email protected] https://lists.gnu.org/mailman/listinfo/gnustep-dev
