Ticket #682 (closed defect: fixed)

Opened 4 years ago

Last modified 16 months ago

objc:defmethod catches harmless conditions

Reported by: gz Owned by: gb
Priority: major Milestone:
Component: Objective-C Bridge Version: trunk
Keywords: Cc:

Description

and turns non-error conditions into errors (making it impossible to use the condition system except for errors).

? (define-condition note-change () ())
NOTE-CHANGE
? (signal 'note-change)
NIL
? (objc:defmethod (#/frotz  :void) ((self ns:ns-application)) (signal 'note-change))
|-[NSApplication frotz]|
? (#/frotz gui::*nsapp*)
> Error: Condition #<NOTE-CHANGE  Condition  #x3020014520AD>
> While executing: #<Anonymous Function #x3000012B946F>, in process Listener(6).
> Type cmd-. to abort, cmd-\ for a list of available restarts.
> Type :? for other options.
1 > 

Change History

comment:1 Changed 4 years ago by gb

  • Owner set to gb
  • Status changed from new to assigned

You can't in general THROW out of a callback:

(defcallback foo (:void)
  (throw 'foo 17))

(catch 'foo
  (#_somethingThatEventuallyCalls foo))

might appear to work, but you don't really know (and can't generally know) what foreign exit points (#_setjmp, exception-handling mechanisms) you just threw past or what the consequences of throwing past them are.

ObjC has fairly well-defined exception-handling mechanisms, and #/ and objc:defmethod conspire to try to unwind both stacks to a point where errors can be handled or re-signaled outside of any pending ObjC callbacks. The mechanism doesn't differentiate between errors and other types of conditions (leading to the behavior described above), but that's only part of the issue.

There's a handler (on CONDITION) effectively wrapped around the callback body; that's a necessary part of the unwinding mechanism, but it causes SIGNAL to believe that there's something interested in that arbitrary condition and to transfer control to that (even though there's no "real" handler for that harmless condition.) If there had been a real handler somewhere up the stack, we would have wanted to do what the handler around the callback does (to act as a barrier to ensure that the foreign stack gets unwound correctly on the way to the real handler.)

It's possible that there's another way to implement that barrier (to force unwinding to happen correctly without the barrier advertising itself as a condition handler), but it's not clear what that other way would be.

comment:2 Changed 4 years ago by gz

Not all condition handlers take a non-local exit. For subclasses of cl:error, most likely yes, but for non-error condition classes, most likely no. I.e., even if there had been a handler for the harmless condition somewhere up the stack, it most likely wouldn't want to unwind the stack, just take some note of the condition having been signalled and let the computation continue.

Perhaps non-error conditions should not try to be interpreted as objc exceptions. There might be a close-enough match between ObJC's exception-handling mechanisms and the lisp error system, but it doesn't seem close enough for non-error condition. A non-error condition handler that does a non-local exit is not any more likely than any old function doing a non-local throw, so if they behaved the same (crash or whatever), it would be better than the current situation where the more likely (and useful) case, non-error conditions that don't abort, are unusable in callbacks.

? (objc:defmethod (#/DoWarn :void) ((self ns:ns-application))
    (warn "No method!"))
|-[NSApplication DoWarn]|
? (#/DoWarn gui::*nsapp*)
> Error: No method!
> While executing: #<Anonymous Function #x3000012B946F>, in process Listener(6).
> Type cmd-. to abort, cmd-\ for a list of available restarts.
> Type :? for other options.
1 > 
? (objc:defmethod (#/TestArray :boolean) ((self ns:ns-application))
    (typep (specifier-type 'maybe) 'array-ctype))
|-[NSApplication TestArray]|
? (#/TestArray gui::*nsapp*)
> Error: #<PARSE-UNKNOWN-TYPE unknown type MAYBE>
> While executing: #<Anonymous Function #x300001BFD40F>, in process Listener(6).
> Type cmd-. to abort, cmd-\ for a list of available restarts.
> Type :? for other options.
1 > 
? (objc:defmethod (#/DoCompile :void) ((self ns:ns-application))
    (compile-user-function '(lambda () *unbound-variable*) 'foo))
|-[NSApplication DoCompile]|
? (#/DoCompile gui::*nsapp*)
> Error: In FOO: Undeclared free variable *UNBOUND-VARIABLE*
> While executing: #<Anonymous Function #x3000012B946F>, in process Listener(6).
> Type cmd-. to abort, cmd-\ for a list of available restarts.
> Type :? for other options.
1 > 

comment:3 follow-up: ↓ 4 Changed 4 years ago by gb

In OBJC:DEFMETHOD (in objc-runtime.lisp), the expansion contains:

                (defcallback ,impname ( :error-return (condition objc-callback-error-return) ,@(arglist))

If you change CONDITION in that form to ERROR (or some other subtype of CONDITION), then only those condition types will be turned into NSExceptions.

I don't remember the details and am too lazy to look at the code, but there's likely some sort of handler established by the event thread at startup; if it wants to handle CONDITION and we don't do OBJC-CALLBACK-ERROR-RETURN on CONDITION, then we'll blissfully neglect to unwind the foreign stack on our way to that handler.

In x86 CCL, "the foreign stack" and "the lisp stack" are different stacks; I have a lot of difficulty believing that failing to unwind the foreign stack is better than anything. It might be better to have WARN print a message and return than to have it do a non-local exit and have something wonder why, but trying to establish handlers for conditions that can't unwind the stack correctly isn't a good idea.

Maybe the right thing is for OBJC:DEFMETHOD to establish a handler for conditions of type (SATISFIES WILL-THROW-SOMEWHERE), where WILL-THROW-SOMEWHERE looks to see if there's a real handler up the stack; if so, it returns T and causes the foreign unwinding to occur on the way to that handler, and if not, SIGNAL doesn't think that there's a real handler around the method.

(That's probably not implementable, but we ideally want something like that effect from SIGNAL: if there's a "foreign unwinding handler" on the way to a real handler, we want SIGNAL to transfer control to it, and that seems to be at least part of the behavior we want. If that is implementable, then we don't have to choose between confusion - why is a WARNING signaled as an error ? - and catastrophe.)

comment:4 in reply to: ↑ 3 Changed 4 years ago by gz

Does the unwinding have to happen at SIGNAL time? Is there any way it could be done with something like an UNWIND-PROTECT around the objc:defmethod?

comment:5 follow-up: ↓ 7 Changed 4 years ago by gb

When an UNWIND-PROTECT cleanup is called, some information used by THROW/NTHROW is saved somehere: there's an extra frame or two on some stacks that contains information about what values are being thrown, about how many more catch frames there are between the current and target context, and about where to return to (e.g., the return address in the call to nthrow.) If you could:

a) tell that you didn't just fall into the cleanup form, e.g., that you're really throwing. (The protected form sets a "done" flag, perhaps very atomically.)

b) find that info and encapsulate it as a lisp object

then you could encapsulate that lisp object in an NSException and raise the NSException.

On the other side of things, #/ basically establishes an NSException handler; if it sees an NSEncapsulatedLispThrow exception, it uses the info to resume the throw.

The details are left as an exercise, but if we can safely throw across foreign code (and foreign exception handlers), we wouldn't see ... the weirdness that arises from believing that we can't do that.

There's some weird low-level funkiness involved in actually doing this, but I'm not sure that it's a whole lot weirder than other stuff that happens in the bridge. The weirdness only happens if a control transfer happens; most of the time, the cleanup form would do nothing.

I like that idea.

comment:6 Changed 4 years ago by gz

(In [13700]) As an interim measure, add compile-time parameter *objc-error-return-condition* to control which conditions gets caught by objc:defmethod. See ticket:682

comment:7 in reply to: ↑ 5 Changed 16 months ago by gb

Replying to gb: [The problem is that we have to be careful about THROWing through foreign code that may have its own cleanup to perform; trying to solve that problem by establishing condition handlers (for CONDITION or ERROR or ...) just introduces another set of problems. It ideally should be possible to THROW out of a callback and have any intervening UNWIND-PROTECTs or ObjC "finally" exception cleanup code run.]

In other words, nothing's changed in the last ~3 years ...

comment:8 Changed 16 months ago by gb

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [15540]) Add a functional interface to THROW (%THROW).

Add %THROWING-THROUGH-CLEANUP-P, which tries to determine whether an UNWIND-PROTECT clanup was invoked in response to THROW or "just fell in". (Both of these functions are currently only implemented on x86; they're primarily intended to be used in the ObjC bridge, and the bridge is currently effectively x86-only.)

Add a :PROPAGATE-THROW option to DEFCALLBACK; it and the :ERROR-RETURN option are incompatible. The :PROPAGATE-THROW option can be used to specify a function which can arrange that foreign cleanup code (as established by ObjC exception handlers) is run on attempts to "throw through them".

Make the ObjC bridge use the new mechanism to ensure that throwing through ObjC handlers works without affecting condition handling in methods defined by OBJC:DEFMETHOD.

Warn on first use of some deprecated ObjC bridge constructs (SEND etc.)

Fixes ticket:682 in the trunk.

Note that there may be code which has depended on the old behavior (and that code could include the CCL IDE's handling of exception on the event thread.) Standard CL condition-handling facilities should work a lot better in the presence of ObjC callbacks now, and using those facilities is likely the best approach to dealing with any problems that arise.

Note: See TracTickets for help on using tickets.