As a hopeful final follow-up to this. I opened a bug
https://savannah.gnu.org/bugs/index.php?41111 with a GNUstep style failing
test that I verified fails on 1.24.6 on Ubuntu along with the fix I
mentioned above. Hopefully it'll be integrated or someone else will be
inspired by a failing test to fix it.
On Tue, Dec 17, 2013 at 2:12 AM, David Chisnall <thera...@sucs.org> wrote:
> The new version still looks more complex than the equivalent (correct)
> code in libobjc2.
>
> David
>
> On 16 Dec 2013, at 23:49, Doug Warren <doug.war...@gmail.com> 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 <thera...@sucs.org>
> 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 <doug.war...@gmail.com> 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 <doug.war...@gmail.com>
> 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 <thera...@sucs.org>
> 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 <doug.war...@gmail.com> 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 <doug.war...@gmail.com>
> 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 <
> david.chisn...@cl.cam.ac.uk> 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 <
> david.chisn...@cl.cam.ac.uk> 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 <doug.war...@gmail.com>
> 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
> > > > >> Gnustep-dev@gnu.org
> > > > >> https://lists.gnu.org/mailman/listinfo/gnustep-dev
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > Gnustep-dev mailing list
> > > > > Gnustep-dev@gnu.org
> > > > > https://lists.gnu.org/mailman/listinfo/gnustep-dev
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > > -- Sent from my brain
> > >
> > >
> > >
> > > _______________________________________________
> > > Gnustep-dev mailing list
> > > Gnustep-dev@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/gnustep-dev
> >
> > --
> > This email complies with ISO 3103
> >
> >
> > _______________________________________________
> > Gnustep-dev mailing list
> > Gnustep-dev@gnu.org
> > https://lists.gnu.org/mailman/listinfo/gnustep-dev
>
>
> -- Sent from my IBM 1620
>
>
_______________________________________________
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
