Issue 329: strange behaviour when Proceeding or Stepping in a newly opened Debugger

http://code.google.com/p/pharo/issues/detail?id=329

what is the status?
Can we make progress on this point?

Stef


Reported by jfitzell, Nov 11, 2008
Summary
~~~~~~~

When you Proceed in a Debugger, it assumes the interruptedProcess is
sitting at the right context and so merely resumes the process. This
assumption is valid once you have hit Restart or recompiled a method but
not (currently) when the Debugger is first opened.

When the Debugger first opens, the interruptedProcess is sitting in
Process>>suspend and that is within the contexts of all the UnhandledError
handling code, the code to open the debugger, etc. So if you hit Proceed
right after opening the Debugger, the behaviour is different (see below).

Current Behaviour
~~~~~~~~~~~~~~~~~

If you evaluate "Error signal" and then immediately Proceed, the behaviour
is as follows:

+ Debugger class>>openOn:context:label:contents:fullView: is resumed and
runs to completion, falling off the end
+ UnhandledError>>defaultAction eventually returns (and returns a Process
because of the weird implementation of StandardToolSet>>debugError:)
+ The (non-resumable) UnhandledError is resumed with that value (a Process)
+ This causes the signal of UnhandledError in Error>>defaultAction to
return the Process
+ Error>>defaultAction now returns but it ignores the Process that was
returned by the UnhandledError and just returns self
+ The (possibly non-resumable) original error is now resumed with the value
of #defaultAction (self)

Expected Behaviour
~~~~~~~~~~~~~~~~~~
While the above basically works, the return value (the Error) is pretty
sub-optimal and it is logically a bit confusing because the Debugger is
supposed to be debugging the context where the error was signaled, not the
code path that led to opening the Debugger.

When you press Step, you expect it to make one step withing the current
context, not to unwind a whole bunch of error handling code first.

I expect step to simply make a step in the signaling context and I expect
Proceed to carry on running in the signaling context. Since we are dealing
with an Error that did not have a default value (since #defaultValue
resulted in opening a Debugger), it seems reasonable to return nil as the
result of the signal.

Reproducing the Problem
~~~~~~~~~~~~~~~~~~~~~~~
This is of course hard to trace. You can verify the return value by simply
inspecting "Error signal" and then hitting Proceed in the Debugger that
pops up. You will see the result is an Error, not nil as I would expect.
(For comparison, VW produces nil in this case, though you have to "Debug"
and then "Run" because it won't let you Proceed a non-resumable error).

To confirm the execution path, I ended up adding Transcript messages in
UndefinedObject>>handleSignal: like this:

handleSignal: exception
  value := exception defaultAction.
  Transcript show: exception name;
    show: ' -- defaultAction returned value: ';
    show: value printString; cr.
  ^ exception resumeUnchecked: value

Suggested Fix
~~~~~~~~~~~~~
As far as I can tell, the best/easiest fix is to have the Debugger set the
process into the debugged context on startup. I did this by making the
attached change to Debugger>>process:controller:context:isolationHead:. It
just adds a call of #popTo:value: on the process. (the debugger doesn't
seem to be in a Monticello package?)

This exception stuff gets complicated though, so discussion may be
warranted if anybody else has a better suggestion.

***As an aside, I think the return values in UnhandledError>>defaultAction
and StandardToolSet>>debugError: should be fixed to be more meaningful (or
self). Returning a Process as the result of either message is silly.



        Debugger-processcontrollercontextisolationHead.st
704 bytes   Download

Delete commentComment 1 by jfitzell, Nov 11, 2008
Sorry... don't know why I couldn't see the Tools package. I found it and saved a
version into Pharo-Inbox:

Name: Tools-jf.90
Author: jf
Time: 11 November 2008, 2:52:23 pm
UUID: 5b6889f2-9071-314b-9c30-e6d83e014049
Ancestors: Tools-stephane.ducasse.89

Possible fix for
issue 329: http://code.google.com/p/pharo/issues/detail?id=329


When opening a Debugger, pop the interruptedProcess back to the context that signaled
the original Error.

Delete commentComment 2 by jfitzell, Nov 11, 2008
Gah! Just discovered my fix seems to break things when a Warning is triggered... I
need to look into it further.

Delete commentComment 3 by jfitzell, Nov 11, 2008
Ok, the error I was getting was "the active process cannot pop contexts". The problem
seems to be fixed by implementing ToolSet>>debugContext:label:contents: as follows:

debugContext: aContext label: aString contents: contents
  ^ self debug: Processor activeProcess context: aContext label: aString contents:
contents fullView: false

This also has the side-benefit that "[Warning signal: 'foo'] fork" will now work (you
get a white screen of death without that change).

Anybody know why the Debugger class-side methods
#openOn:context:label:contents:fullView: and #openContext:label:contents: are
implemented differently to begin with? The method names don't suggest that their
behaviour should be different.

I feel like I've pulled on a thread that maybe I shouldn't have pulled on... :)

Delete commentComment 4 by jfitzell, Nov 11, 2008
And I notice DebuggerUnwindBug>>testUnwindDebuggerWithStep is failing, but it seems
to fail even with my changes reverted so I think that's not a problem here.

Delete commentComment 5 by marcus.denker, Feb 18, 2009
As the change seems to be not stable yet, I have attached the mcz to the bug-report (this way we can clean the
inbox)


        Tools-jf.90.mcz
384 KB   Download
_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project