views:

209

answers:

4

If I release an instance of NSOperation before sending -init to it I get a segmentation fault.

Reasons I think this is valid code:

  • Apple does this in its documentation.
  • Gnustep does it in its implementation of NSNumber, so it's fairly certain that this is in Apple's code too. (At least was.)
  • NSObjects -init doesn't do anything, therefore -release, which belongs to NSObject should be working before that.
// gcc -o test -L/System/Library/Frameworks -framework Foundation test.m

#import <Foundation/Foundation.h>

int main(int argc, char *argv[]) {
    NSOperation *theOperation = [NSOperation alloc];
    [theOperation release];
}
  • What do you think, is this a bug?
  • Can you show me an example of another class that has the same behavior?
  • Any idea why this is happening?
+5  A: 

Sending any message other than init to an object that has not been initialized is not valid code AFAIK. Call the superclass initializer and then release and I'm betting it won't crash (although having one class's initializer return a completely unrelated class strikes me as doubleplusungood).

Chuck
Correct answer. More details in mind. I actually compiled a test project to make sure this wasn't a latent bug somewhere. It isn't.
bbum
Returning a completely unrelated class is just for the example. I'm not doing this in my working code. Gnustep's `NSNumber` class also sends `-release` prior to initialization. `-initWithCoder` never calls `-init` as far is I know.
Georg
`initWithCoder:` methods do normally call through to super.
Chuck
Apple does it in its documentation, that makes it valid I guess: http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/ObjectiveC/Articles/ocAllocInit.html#//apple%5Fref/doc/uid/TP30001163-CH22-SW13
Georg
No, that makes Apple's documentation very slightly flawed. bbum demonstrated that doing as I said got rid of the crash.
Chuck
Or Apple's implementation of NSOperation as many classes use this kind of code construct.
Georg
+2  A: 

There is nothing remotely valid about that code.

Rewrite your -init as:

- (id) init
{
    if (self = [super init]) {
        [self release];

        NSNumber *number = [[NSNumber alloc] initWithInteger:6];
        return number;
    }
    return self;
}

Of course, the code is still nonsense, but it doesn't crash.

You must always call through to the super's initializer before messaging self. And you must always do it with the pattern shown above.

bbum
I changed the code to avoid more discussion about the code. Releasing before initializing is valid: http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/ObjectiveC/Articles/ocAllocInit.html#//apple%5Fref/doc/uid/TP30001163-CH22-SW13
Georg
+1  A: 

I thought you need to initialize your superclass before calling release, but according to this example in Apple's docs that's not the case.

So it may be a bug, but certainly not an important one.

Darren
I think you've found a documentation bug.
Chuck
+1  A: 

My previou analysis was not really correct.

However, I want to point that this issue can happen with different classes. It actually depends on which class you're subclassing. Subclassing NSObject is no problem, but subclassing NSOperation, NSOperationQueue, and NSThread is an issue, for example.

And it happens because just like YOU might do, the classes you subclass might allocate stuff in their -init method. And that's also where you set to nil the variables you have not allocated yet (and might do so later in your code).

So, by calling -release on yourself without a previous -init, you might cause one of your parent classes to release object that it had not allocated. And they can't check if their object is nil, because it didn't even have the opportunity to init every object/value it needs.

This also might be the reason why releasing NSOperation without init worked on 10.5 and doesn't work on 10.6. The 10.6 implementation have been rewritten to use blocks and Grand Central Dispatch, and thus, their init and dealloc methods might have changed greatly, creating a different behavior on that piece of code.

naixn
Sending `-release` to a nil object shouldn't do anything. Also, freeing `NULL` pointers is supposed to do nothing. http://www.opengroup.org/onlinepubs/009695399/functions/free.html Therefore, I think this is a bug.
Georg
@gs: Iknow free() and release work on NULL objects. However, when exactly do you want them to set the pointer to NULL / nil when you don't even let them init their object? (which is my point in this answer). Allocating the pointer on the stack doesn't set it to NULL / nil. That's why I think it's NOT a bug.
naixn
Ivars are automatically set to nil in alloc.
Chuck
@Chuck: what about pure C attributes? Like structuers etc.? :)
naixn
They don't have to be freed in `-dealloc` as they are automatically removed when the object is deallocated. They work just like integers and the like.
Georg
@gs: simple structures, yes. But not pointers to structures. These are not freed on `-dealloc`, nor set to `NULL` without init.
naixn
Yes it does. http://snipt.org/nllg
Georg
You're right, my comment was based on pure C. I didn't know the Obj-C runtime set all ivar pointers to 0. That's good to know. My only last guess is that the documentation is wrong, and that even people at Apple are assument that if they go to through their `-dealloc` method, then it "obviously" went through `-init`. And then, all they need to do is `struct->value` and bam, segfault.I guess you can file a bug, they will either fix the code, or the documentation :)
naixn
It actually crashes in `-release` and not in `-dealloc`, that's what surprised me too.
Georg
It is not impossible that Apple maintain an internal reference counting, as a kind of semaphore for example. Especially knowing the crash happens with NSOperationQueue, NSOperation, and NSThread :D.
naixn