armadsen / ORSSerialPort

Serial port library for Objective-C and Swift macOS apps
MIT License
752 stars 183 forks source link

Crash during dealloc #133

Closed carlos4242 closed 5 years ago

carlos4242 commented 5 years ago

I'm seeing an occasional crash with an app that's been running some time.

The crash is here:

- (void)notifyDelegateOfPosixErrorWaitingUntilDone:(BOOL)shouldWait;
{
    if (![self.delegate respondsToSelector:@selector(serialPort:didEncounterError:)]) return;   

In this case the delegate has been deallocated.

I've tried to write protection code to reset the delegate but really a delegate should always be a weak reference anyway.

The delegate is declared like this:

#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_8
@property (nonatomic, weak, nullable) id<ORSSerialPortDelegate> delegate;
#else
@property (nonatomic, unsafe_unretained, nullable) id<ORSSerialPortDelegate> delegate;
#endif

For some reason this isn't working. I'm not very familiar with these macros. Are they used wrong?

armadsen commented 5 years ago

They are used correctly, but it's a little complicated. MAC_OS_X_VERSION_MIN_REQUIRED is set to the deployment target set for the project. If you're including ORSSerialPort in your project as a framework (as a subproject, via Carthage, or possibly using CocoaPods in a Swift project or with use_frameworks! set), the deployment target is the one set in ORSSerialPort.xcodeproj (10.7 as of this writing), not the one set in your app project.

I've opened an issue to update the deployment target to 10.8, which will mean we can make ORSSerialPort.delegate unconditionally weak. In the meantime, the easiest workaround is probably just to bump the deployment target of ORSSerialPort.framework itself to 10.8 (or whatever target you are actually deploying to). You could do this in your own fork if you want.

(Boring backstory: weak wasn't available when ORSSerialPort was first written (non-ARC!), and even after the transition to ARC: a) wasn't available when deploying to 10.6 b) wasn't supported by a number of classes that were likely delegates of an ORSSerialPort, most notably NSWindowController and NSViewController. Even now, ORSSerialPort technically supports deployment back to 10.7 which still doesn't allow NSWindowController or NSViewController to be the target of a weak reference.)

armadsen commented 5 years ago

Closing in favor of #137 to track a fix for this.