UnhandledException handler in the "Interact" evaluator in the inspector

[Eliot Miranda]

Hi All,

    the UnhandledError handlers in the evaluators break the debugger.  e.g.

ObjectSubject>>evaluate: expression ifCompilerError: onCompilerError <[String]> ifError: onError <[NewspeakDebugging ThreadSubject, Exception]> ^<ObjectMirror> = (

| compilerError failed sem result process |

failed:: false.

compilerError:: nil.

sem:: Semaphore new.

process::

[[result:: objectMirror evaluate: expression ifError: [:message | compilerError:: message]. sem signal] on: UnhandledError do:

[:ex |

failed:: true.

result:: ex exception.

sem signal.

process suspend]

] newProcess.

process

name: 'Evaluating ', expression printString;

resume.

sem wait.

process offList.

compilerError ifNotNil: [:it | ^onCompilerError valueWithPossibleArgument: it].

failed ifTrue:

[^onError value: (finalizer subjectFor: result in: process) value: result].

^result

)

If one tries to debug an expression within this context, all attempts at stepping end up cutting back the stack to this handler *unless* one hacks ContextPart>>runUntilErrorOrReturnFrom:.  This hack breaks the debugger's handling of block non-local return.

For example, reproduce thusly:

1. check if ContextPart>>runUntilErrorOrReturnFrom: contains my hack, which reads something like

[ctxt isDead or: [aSender isDead]] whileFalse: [topContext := topContext stepToCallee].

^{aSender isDead

ifTrue:

[| retValue |

retValue := ctxt isUnwindContext ifTrue:

[ctxt tempAt: 3]. "returnValue in ensure: and result in ifCurtailed:"

aSendersSender push: retValue.

aSendersSender]

ifFalse: [topContext].

    nil}

  the phrase should read simply

[ctxt isDead] whileFalse: [topContext := topContext stepToCallee].

^{topContext. nil}

If it contains the hack you can see that stepping over the value: send in Interval>>do: in e.g. self halt. (1 to: 5) do: [:i| i = 1 ifTrue: [^#ok]]. ^#bad does *not* return.  Remove the hack and see that the debugger does execute the non-local return. 

So without the hack evaluate e.g.

self halt. (1 to: 5) do: [:i| i = 1 ifTrue: [^#ok]. #bad]

In the resulting debugger inspect the Halt context.  In that inspector use Interact to get an evaluator in which one writes e.g.

self halt. self interpretNextInstructionFor: self

Clock on the Halt exception to open a debugger, and therein try any kind of step and the debugger will jump back to the UnhandledError handler in ObjectSubject>>evaluate: ifCompilerError ifError:.

IMO the UnhandledError handler in ObjectSubject>>evaluate:ifCompilerErrorifError: is erroneous. What happens is that when the Halt is raised, the handler catches the exception, allowing it to be passed to the debugger, but the continuation is to return to the enclosing block (the block that receives newProcess in ObjectSubject>>evaluate:ifCompilerErrorifError:.

One question is what was the intent of the UnhandledError handler in the above?  Could it be that not all compiler errors were reported through the ifError: arm of evaluate:ifError:?  If so, the right fix is to refactor evaluation to split compilation of the doit from execution of the doit, wrap the compilation with an Error handler, and leave the doit's execution unguarded.

Another question is if I make some changes is this likely to affect the JavaScript or Dart back-ends?

--
best,

Eliot

[Ryan Macnak]

On Fri, Jul 12, 2013 at 1:41 PM, Eliot Miranda <eliot....@gmail.com> wrote:

    the UnhandledError handlers in the evaluators break the debugger.  e.g.

One question is what was the intent of the UnhandledError handler in the above?  Could it be that not all compiler errors were reported through the ifError: arm of evaluate:ifError:?  If so, the right fix is to refactor evaluation to split compilation of the doit from execution of the doit, wrap the compilation with an Error handler, and leave the doit's execution unguarded.

The version of ObjectSubject>>eval in the bleeding branch that does not involve UnhandledError. I hope to merge your recent putback this weekend.

  

Another question is if I make some changes is this likely to affect the JavaScript or Dart back-ends?

They don't have ObjectMirror>>eval, so no. 

Ryan

[Eliot Miranda]

Hmmm, my last message was missing something.  The intent of the UnhandledError handler is to catch the exception in the context of the inspector, presenting is as a link that can be clicked to open a debugger on the exception.  Without the UnhandledError handler the exception opens a new debugger.  So we *do* need the UnhandledError handler.  However, a) the continuation is wrong, and b) the handler should not fire more than once.  So the correct implementation (presumptuous, ed.) is as follows, with modifications underlined.

evaluate: expression  ifCompilerError: onCompilerError <[:String]> ifError: onError <[:NewspeakDebugging ThreadSubject :Exception]> ^<ObjectMirror> = (

| compilerError failed sem result process |

failed:: false.

compilerError:: nil.

sem:: Semaphore new.

process::

[

[(* Note that the ObjectMirror used by the inspectors is still the old one in SqueakVmMirror, hence the annoying unpacking/repacking between different ObjectMirrors. *)

objectMirror reflecteeClass language isNewspeakLanguage3

ifTrue: [result:: objectMirror class on: 

((ObjectMirror reflecting: objectMirror reflectee)

evaluate: expression

withBlackMarket: blackMarket) reflectee]

ifFalse: [result:: objectMirror

evaluate: expression

ifError: [:message | compilerError:: message]].

sem signal] 

on: UnhandledError

do: [:ex |

failed ifTrue: [ex pass]. (* don't fire twice *)

failed:: true.

result:: ex exception.

sem signal.

process suspend]

] newProcess.

process

name: 'Evaluating ', expression printString;

resume.

sem wait.

process offList.

compilerError ifNotNil: [:it | ^onCompilerError valueWithPossibleArgument: it].

failed ifTrue:

[(* position the process to continue in the signalerCOntext. *)

 process suspendedContext unwindTo: result signalerContext.

result signalerContext push: result.

process suspendedContext: result signalerContext.

^onError value: (finalizer subjectFor: result in: process) value: result].

^result

)

--
best,

Eliot