[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: libobjc2 objc_setassociatedobject issue
From: |
David Chisnall |
Subject: |
Re: libobjc2 objc_setassociatedobject issue |
Date: |
Tue, 17 Dec 2013 10:12:37 +0000 |
The new version still looks more complex than the equivalent (correct) code in
libobjc2.
David
On 16 Dec 2013, at 23:49, Doug Warren <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> 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
> > > >> address@hidden
> > > >> https://lists.gnu.org/mailman/listinfo/gnustep-dev
> > > >
> > > >
> > > > _______________________________________________
> > > > Gnustep-dev mailing list
> > > > address@hidden
> > > > https://lists.gnu.org/mailman/listinfo/gnustep-dev
> > >
> > >
> > >
> >
> >
> >
> >
> > -- Sent from my brain
> >
> >
> >
> > _______________________________________________
> > Gnustep-dev mailing list
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/gnustep-dev
>
> --
> This email complies with ISO 3103
>
>
> _______________________________________________
> Gnustep-dev mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/gnustep-dev
-- Sent from my IBM 1620
- Re: libobjc2 objc_setassociatedobject issue, David Chisnall, 2013/12/13
- Re: libobjc2 objc_setassociatedobject issue, David Chisnall, 2013/12/13
- Re: libobjc2 objc_setassociatedobject issue, Doug Warren, 2013/12/13
- Re: libobjc2 objc_setassociatedobject issue, Doug Warren, 2013/12/13
- Re: libobjc2 objc_setassociatedobject issue, David Chisnall, 2013/12/13
- Re: libobjc2 objc_setassociatedobject issue, Doug Warren, 2013/12/13
- Re: libobjc2 objc_setassociatedobject issue, Doug Warren, 2013/12/16
- Re: libobjc2 objc_setassociatedobject issue, David Chisnall, 2013/12/16
- Re: libobjc2 objc_setassociatedobject issue, Doug Warren, 2013/12/16
- Re: libobjc2 objc_setassociatedobject issue,
David Chisnall <=