cc @slavapestov

So the immediate regression here is that swift_deallocPartialInstance() doesn't handle non-pointer isa, but in fact the old code was wrong too – it only looked like it worked when asserts were disabled. We would just go ahead and use a swift reference counting operation on an Objective-C object, which just clobbers some random memory before calling free().

@belkadan, should we just hack around this by directly calling free()?

We can't guarantee that +alloc returned memory that came from malloc unless we emit a +alloc for any Swift subclass that has a failable init, but maybe it's close enough. Any such class would also have to override -dealloc, I think, and that could cause all sorts of havoc. @gparker42?

Yeah, I think we already had this discussion and the conclusion was that we would have a 'whitelist' of types that cannot allow -release without -init (like NSTimer), and call free() or -dealloc directly for those. In the future, we should try to define a proper protocol for this type of thing.

But in the short term, what would be a reasonable fix? It seems unfortunate that assertions enabled breaks something that used to "work", even if "work" wasn't perfect.

Wait, how is this a regression from Swift 2.1? I thought we didn't support return-before-super until 2.2.

(@jckarter should probably be in this conversation too, especially if Greg's around.)

We had supported early exit from convenience initializers, since there's no partial-deinitialization problem in that case.

We support 'return before self.init' in convenience inits in 2.1. Here the instance is not partially initialized, it's completely uninitialized – so for Swift objects, we can just free the memory.

For Objective-C objects, this works on accident if assertions are off, because swift_deallocObjectInstance() crashes if the object doesn't have a Swift refcount.

So it sounds like people were relying on this behavior in 2.1, with Objective-C objects, which is unfortunate.

It definitely seems like a regression that we'd try to deallocPartialInstance after a convenience initializer failure.

Joe, deallocPartialInstance() takes a metaclass and an instance; the metaclass is the most derived class for which the instance is not initialized.

Convenience init failure before self.init() will call deallocPartialInstance() with the object and its isa. This effectively means it is completely uninitialized. This is the same behavior as before, except we had a special entry point for that.

Convenience init failure after self.init() just releases the instance as you would expect.

Oh, I guess the one regression is that deallocPartialInstance() doesn't support non-pointer isa. If we fix that, then an asserts-disabled build will have the same behavior as Swift 2.1. Should we do that?

That sounds like a good bug to fix regardless. Preserving the 2.1 behavior seems like a good short-term plan for 2.2.

Alright. I'll fix the non-pointer isa thing, and also have it skip the assertion if we have an objc instance. That will give us the 2.1-without-asserts behavior in 2.2-with-asserts builds. For 3.0 we can look into a more comprehensive fix involving changing the class of the instance and sending a -release.

I just ran into this as well. FWIW, you can make the reproducible case even smaller:

import Foundation

extension NSObject {
    convenience init?(value: Int) {
        guard value < 5 else { return nil }
        self.init()
    }
}

if let c = NSObject(value: 5) {
    print("got \(c)")
}

Comment by Vlas Voloshin (JIRA)

As of Xcode 7.3 (7D175) with Swift 2.2 (swiftlang-703.0.18.1 clang-703.0.29), this issue no longer occurs with reproduction scenario in the issue description. However, scenario posted above by @jtbandes with a failable convenience initialiser on NSObject would still crash. Defining a failable convenience initialiser for an NSObject subclass in an extension and calling it doesn't crash though. Maybe base NSObject class is a special case?

Comment by Christopher Bryan Henderson (JIRA)

An initializer that throws is also broken. Code below crashes with release version of Xcode 7.3:

enum MyError: ErrorType {
    case Example
}

extension UIColor {
    convenience init(fail: Bool) throws {
        guard !fail else {
            throw MyError.Example
        }
        self.init(red: 1, green: 1, blue: 1, alpha: 1)
    }
}

let color = try? UIColor(fail: true)

Comment by Silver (JIRA)

This is now breaking things on the latest stable XCode.

What's the proposed workaround until this is fixed?

FWIW, this was fixed on mainline by

3fb06d7

The workaround is to only return nil after the super.init.

I've created a PR against Swift 2.2 for this:

#1798

Comment by Cal S (JIRA)

+1 that the issue can be avoided by calling self/super.init(); before retuning nil.

I am also seeing this issue with a non-failable but throwing convenience initializer in an NSData extension. In this case, calling self.init before throwing did not prevent the crash. However, when I put all the code (which includes some closures and a while loop) into a helper function that I call from the initializer, the self.init workaround worked.

Michael, that's almost certainly the same issue: any kind of early exit from the initializer of an Objective-C-rooted class will run into this crash in the runtime.

Doug's 2.2.1 PR was accepted.