Crasher in Swift<->Objective-C Interop!


(Charles Srstka) #1

Hello all,

I’ve discovered a bug that’s causing crashes when Swift code interoperates with AppKit in a certain way. I intend to file a bug report, but I’m not sure whether the bug should be filed as a Swift bug, or as an AppKit bug since a case could probably be made for either, and I thought I would ask the list first.

What’s happening, basically, is that when an NSTextFieldCell subclass contains Objective-C-compatible reference-type properties, and is loaded from a .nib file in which its containing text field has its Baseline aligned to some other object via autolayout, its properties get over-released when the cell is deallocated. A simple example that will cause the crash is:

@objc(Foo) class Foo: NSObject {}

@objc(CustomTextFieldCell) class CustomTextFieldCell: NSTextFieldCell {
    let foo = Foo()
}

The equivalent code in Objective-C works properly and does not crash:

#import <Cocoa/Cocoa.h>

@interface Foo: NSObject
@end

@implementation Foo
@end

@interface CustomTextFieldCell: NSTextFieldCell

@property (nonatomic, strong) Foo *foo;

@end

@implementation CustomTextFieldCell

- (instancetype)initWithCoder:(NSCoder *)coder {
    self->_foo = [Foo new];
    
    return [super initWithCoder:coder];
}

@end

The problem seems to occur, as far as I can tell, because when the Baseline layout relation is applied, AppKit copies the text field’s cell. Subsequently, NSCell’s -copyWithZone: method calls NSCopyObject, which in turn calls a private function named “fixUpCopiedIvars.” With an Objective-C cell class, fixUpCopiedIvars calls class_getIvarLayout, and retains all its instance variables, so both the original cell and the copy have an owning reference to all of them. This retain is then balanced by a release when the cell is deallocated. With a Swift cell class, however, class_getIvarLayout returns NULL, so the ivars are never retained; however, this nonexistent retain is still balanced by a release when the cell is deallocated. The result is that the program accesses freed memory, leading to a crash or worse.

A sample project demonstrating all this is here: http://www.charlessoft.com/bug_examples/Crash_Swiftly.zip

So, there’s clearly a bug here, but I’m not sure which of these three possibilities is correct:

- This is a bug in AppKit, because NSCell should not be using the deprecated NSCopyObject or assuming that class_getIvarLayout will work.

- This is a bug in the Swift<->Objective-C bridge, because Swift can be used to write Objective-C objects, and legacy Objective-C code like NSCopyObject that interacts with said objects may assume that it can access instance variables via class_getIvarLayout; thus, the latter should work.

- Or option 3: The Swift team considers it to be a bug in AppKit, and the AppKit team considers this to be a bug in Swift, or maybe the Foundation team gets involved to make this into a triangle. This is obviously the worst-case scenario. :wink:

What does the community think? Should I file this as a bug on Swift? Foundation? AppKit? All three?

Thanks,
Charles


(Brent Royal-Gordon) #2

I would file it against AppKit. Once it's in Radar, they can kick the can over to Foundation or Swift, but it's trickier to do the opposite.

···

On Apr 30, 2017, at 11:57 AM, Charles Srstka via swift-users <swift-users@swift.org> wrote:

What does the community think? Should I file this as a bug on Swift? Foundation? AppKit? All three?

--
Brent Royal-Gordon
Architechies


(Charles Srstka) #3

Thought about it a bit, and it really is two separate issues. AppKit probably *shouldn’t* be using NSCopyObject to copy things, and Swift shouldn’t be crashing if it does. So, I’ve filed a bug report on each:

rdar://31912100
https://bugs.swift.org/browse/SR-4756

In the event that anyone with the same problem (which, if anyone’s curious, was implementing a custom field editor, the only customization point for which is on NSCell rather than NSControl) discovers this thread, let me save you a little time on some workarounds that don’t work:

Implementing your own copy(with:) method fails, because you need to call super’s implementation to copy the cell effectively, and the crash will still occur.

Using a weak property to the containing NSControl and putting your stored properties there doesn't work, because NSCopyObject doesn't seem to play nice with weak variables either, and all sorts of errors will be logged to the console.

Using an unowned property to the containing NSControl doesn't work, because unowned properties cannot be null, and the connection will not be made until runtime.

Making a non-Objective-C property and storing your Objective-C properties inside it seemed to work some of the time, and crashed other times, when I tried it. I haven’t studied the low-level layout of Swift objects enough to know why this was.

What works is to use Objective-C associated objects. Use objc_getAssociatedObject in the getter and objc_setAssociatedObject in the setter. Be sure to use withUnsafeMutableBytes on your key property and pass its baseAddress as the key, because the associated object functions take immutable pointers, and since your key property has to be a var to ensure it stays unique, just passing it via & character will cause Swift to make an immutable copy first, and then pass a pointer to *that*, and things won’t work.

Charles

···

On Apr 30, 2017, at 4:43 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

On Apr 30, 2017, at 11:57 AM, Charles Srstka via swift-users <swift-users@swift.org> wrote:

What does the community think? Should I file this as a bug on Swift? Foundation? AppKit? All three?

I would file it against AppKit. Once it's in Radar, they can kick the can over to Foundation or Swift, but it's trickier to do the opposite.