Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling when a required selector isn't defined #94

Open
freakboy3742 opened this issue Dec 29, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@freakboy3742
Copy link
Member

commented Dec 29, 2017

At present, if a class doesn't implement a required selector, the Objective C runtime raises a hard crash. For example:

2017-12-29 12:09:23.220 python[22410:8656994] -[TogaCanvas window]: unrecognized selector sent to instance 0x7fad6342b1b0
2017-12-29 12:09:23.228 python[22410:8656994] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[TogaCanvas window]: unrecognized selector sent to instance 0x7fad6342b1b0'
*** First throw call stack:
(
	0   CoreFoundation                      0x00007fffaa5022cb __exceptionPreprocess + 171
	1   libobjc.A.dylib                     0x00007fffbf31b48d objc_exception_throw + 48
	2   CoreFoundation                      0x00007fffaa583f04 -[NSObject(NSObject) doesNotRecognizeSelector:] + 132
	3   CoreFoundation                      0x00007fffaa474755 ___forwarding___ + 1061
	4   CoreFoundation                      0x00007fffaa4742a8 _CF_forwarding_prep_0 + 120
	5   AppKit                              0x00007fffa7f4ffcf -[NSView addSubview:] + 59
	6   _ctypes.cpython-35m-darwin.so       0x000000010b9602ef ffi_call_unix64 + 79
	7   ???                                 0x00007fff55020830 0x0 + 140734619584560
)
libc++abi.dylib: terminating with uncaught exception of type NSException

It should be possible to avoid the hard crash by implementing doesNotRecognizeSelector: on the base ObjInstance. This should provide a better opportunity to manage or report the crash than showing a memory dump.

@dgelessus

This comment has been minimized.

Copy link
Collaborator

commented Dec 30, 2017

Hm. I agree that a hard crash in response to a missing method isn't great, but I'm not sure what a better alternative would be.

I don't think it would be a good idea to continue execution normally after a missing method is called. If the caller expected a return value from the missing method, it will now get random garbage. Even if no return value was expected, the method call was meant to have some effect, which was probably required for the following code to work correctly. In any case, the following code will be working with unexpected or garbage data, and is likely to error or segfault (or silently do the wrong thing, which is even worse).

A safer approach would be to let the exception be thrown, but catch it in Rubicon to prevent Python from crashing entirely. However, catching Objective-C exceptions from Python is not easily possible, see #73. An easier alternative is setting an uncaught exception handler using objc_setUncaughtExceptionHandler (or its Foundation equivalent, NSSetUncaughtExceptionHandler). This allows executing custom code after an exception was not caught, but once the handler returns, the process will crash anyway. (At that point the exception has gone up the entire call stack and nothing has caught it, so there is no code that could be safely returned to.)

Somewhat related: in #69 I suggested adding checks to ensure that custom ObjC subclasses implement all methods of protocols they implement. That would prevent "unrecognized selector" errors from incomplete implementations of protocols. I don't think that would help with the example posted here - that looks more like a type error. -[NSView addSubview:] takes a NSView argument, and NSView provides a window property.

@freakboy3742

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2017

I wasn't suggesting a completely silent failure, or even preventing the hard crash - just transforming it into a Python hard crash. Turning an Objective C stack trace caused by a missing selector into a Python AttributeError() would, at the very least make it easier to work out where the problem was caused; as it, that context is almost completely lost. In the example provided, all you know is that you're looking for a call to "[TogaCanvas window]".

@dgelessus

This comment has been minimized.

Copy link
Collaborator

commented Dec 31, 2017

If what you're looking for is a Python traceback to where the crash happened, then the faulthandler module is the easiest solution, and it doesn't even require any changes to Rubicon:

Call faulthandler.enable() to install fault handlers for the SIGSEGV, SIGFPE, SIGABRT, SIGBUS, and SIGILL signals. You can also enable them at startup by setting the PYTHONFAULTHANDLER environment variable or by using the -X faulthandler command line option.

Our test suite already uses this.

Converting the error to a Python exception sadly isn't easily done. It's not possible to use -[NSObject doesNotRecognizeSelector:] for this - it can be overridden to run extra code when a missing method is called, but (like with the uncaught exception handler) at that point it's too late to stop the Objective-C exception from being thrown. It would be possible to hook in earlier in the method lookup/call process, for example using -[NSObject forwardInvocation:], but without throwing an Objective-C exception we still have no way to safely abort the Objective-C code and return to Python.

So in the end I think we're still left with catching the Objective-C exception. As explained in #73, that is annoying to implement, requires compiled code (since ctypes/libffi don't support libunwind exceptions), and will likely be very slow if we do it automatically for every method call.

@dgelessus dgelessus changed the title Improve error handling when a requiredselector isn't defined Improve error handling when a required selector isn't defined Dec 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.