Hi Eliot,


you're genius, thanks a lot for your answer! :-)


First, for convenience only, here is a snippet to reproduce the bug even simpler:


| ctxt |
ctxt := thisContext.
[thisContext swapSender: ctxt] value "step OVER this line to reproduce"

I played a lot around with this issue during the last days, and I could not develop a perfect solution so far. But here are some considerations:

First, what exact behavior do we expect when stepping over some code that includes a sender swap, speaking in general? The current implementation wants to keep the process running until we return from the selected message - which is the desired behavior in most cases. Just in this special case, our problem is that the selected message will never return.
If I understand your goal definition correctly, the debugger would halt each time a sender swap occurs while the over button is pressed? I fear this could be inconvenient if you step over any method from a higher abstraction level that uses a generator in any deep implementation details. So I would like to refine the goal rather as "when pressing over, the process should be resumed until the suspended context is in the stack of the original context (i. e. where the over button was pressed)". For illustration, neglecting the whole optimization, Process >> #complete: should basically equal the pseudocode "self runUntil: [:ctxt | aContext = ctxt or: [aContext hasSender: ctxt]]". Would you agree so far?

Second, my approach was to modify #swapSender: to manually identify and transport these "essential" sender contexts. The attached changeset is really WIP, so it only works for the simplest examples, but it might give you an impression of my approach. It does not yet work properly for the following code, if you step over #contents, for example:

(Generator on: [:stream |
stream nextPut: #foo; nextPut: #bar]) contents.

If you like the idea, I will try to fix that later.
I know that this approach is in O(n) whereas the current implementation has a constant complexity only, but to me, this appears inherent and inevitable complexity.

However, we would need this modification only for debugging. This leads me to my next concern:
>From an architectural point of view, I would like to put this modification into a decorator class (DebuggedContext) that is only installed from #runUntilErrorOrReturnFrom:.
But it appears the VM does not like this, you cannot even do:

thisContext privSender: (thisContext sender changeClassTo: thisContext sender class newSubclass)

(computation has been terminated), unless you are debugging. I'd guess that primitive 160 always allocates new memory for the receiver, which would be invisible for the image, but visible to the VM. But I don't have any clue of the actual implementation! Any chance to get around this limitation?

In general, how would you think about this approach? Do you have any better ideas? Looking forward to your opinion :-)

Best,
Christoph


________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Eliot Miranda <eliot.miranda at gmail.com>
Gesendet: Dienstag, 24. Dezember 2019 03:57:12
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] BUG/REGRESSION while debugging Generator >> #nextPut:

Hi Christophe,

On Fri, Dec 13, 2019 at 10:48 AM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de<mailto:Christoph.Thiede at student.hpi.uni-potsdam.de>> wrote:

Hi all,


I discovered a bug when stepping over a call to Generator >> #nextPut:.

Apologies.  I can now reproduce the bug.  I was stupidly stepping over the block evaluation, not the send of nextPut:.


!! Save your image before trying the following !!


Generator
on: [:stream | stream nextPut: #foo]


[cid:16f35ae515df456b1e51]


  1.  Due to multiprocessing implementation of the Generator, pressing over will hang up the current process forever, because the generator is never queried.
  2.  If I press cmd-dot, I get an infinite number of debugger windows that show Context >> #cannotReturn:. This will probably damage your image.

The first is annoying for newcomers, but I see this behavior is reasonable and without the effect of the second, it wouldn't be a big problem.
The second, however, looks like a bigger issue to me. Actually, I think the most serious aspect is that in Squeak 5.1, the same procedure does not crash your image, but you are forwarded to the emergency debugger and can terminate the process:

[cid:16f35ae515df456b1e52] [cid:16f35ae515df456b1e53]

Also, this is not the first scenario in the last time where I got an infinite debugger chain. See [BUG(s)] in Context control (#jump, #runUntilErrorOrReturnFrom:)<http://forum.world.st/BUG-s-in-Context-control-jump-runUntilErrorOrReturnFrom-td5107263.html> for a similar issue. And there were even more situations which I could not yet reproduce exactly.
I'm afraid that the recent changes to the debuggers might have weakened its ability to detect recursive errors. Can someone else tell about these problems?

I suppose we ignore a large number of recursive errors as in MorphicDebugger >> #openOn:context:label:contents:fullView:, the uiBlock is often triggered as a separate UI message which will be executed after the recursion flag has been cleared by the base class.

Would be great if someone could have a look at it or share more information :)

So the issue seems to me to be the interaction between the block established in runUntilErrorOrReturnFrom: to catch unhandled errors (this one:
Context contextOn: UnhandledError do: [:ex |
error ifNil: [
error := ex exception.
topContext := thisContext.
ex resumeUnchecked: here jump]
ifNotNil: [ex pass]
]
)
and the swapSender: in Generator>>nextPut:.  So after the swapSender: the Generator's continue variable refers to this BlockClosure>>on:do:. BlockClosure>>ensure: pair introduced by runUntilErrorOrReturnFrom:.  What I don't understand was how the evaluateOnBehalfOf: changes interact with that.  But what I can say is that the Debugger is leaving the Generator (or any code that uses swapSender:) in an invalid state because
a) once the swapSender: occurs the protect blocks (the on:do:,ensure: pair) are no longer on the stack.  What the swapSender should have been morphed into doing was preserving the protect blocks under the current context, or rather that swapSender should have been persuaded to swap the senders of the protect blocks.  Now when the debugger continues stepping it never gets to the protect blocks because they have been swapped out of the way, and execution proceeds all the way down to the fork in Generator>>reset.

You can observe this by
- stepping through "Generator on: [:stream | stream nextPut: #foo]" until at the send of #nextPut:
- inspecting the Debugger (via the Morphic tool handle menu "inspect model"
- debugging "self doStep" to step the Debugger through the send
- when you get to runUntilErrorOrReturnFrom: *ONLY STEP UP TO THE jump* *DO NOT STEP OVER THE jump*
- at the jump, step into, then do step into, and you'll find the debugger now in Generator>>nextPut:, with the stack still including the protect pair.  But when you step over the swapSender: then, of course, they've gone missing, and the stack points back to the fork in reset with nothing on the stack to stop execution (because there's no ensure: block which is what runUntilErrorOrReturnFrom: inserted to try and catch any attempt to return).

So to get to the point of the swapSender: you should see this:

Context>>jump
Context>>runUntilErrorOrReturnFrom:
[] in Process>>complete:
BlockClosure>>ensure:
Process>>evaluate:onBehalfOf:
Process>>complete:
Process>>completeStep:
[] in MorphicDebugger(Debugger)>>doStep
BlockClosure>>on:do:
MorphicDebugger(Debugger)>>handleLabelUpdatesIn:whenExecuting:
MorphicDebugger(Debugger)>>doStep

and the stack from the Context about to jump is

Generator>>nextPut:
BlockClosure>>on:do:
BlockClosure>>ensure:
[] in UndefinedObject>>DoIt
Generator>>fork
[] in Generator>>reset
Generator>>reset
Generator>>initializeOn:
Generator class>>on:
UndefinedObject>>DoIt ...

and the Generator's "continue" stack immediately before the swapSender: in Generator>>nextPut: is

Generator>>reset
Generator>>initializeOn:
Generator class>>on:
UndefinedObject>>DoIt

What we'd like to see after the swapSender: is the protect blocks moved to the continue stack:
Generator>>nextPut:
BlockClosure>>on:do:
BlockClosure>>ensure:
Generator>>reset
Generator>>initializeOn:
Generator class>>on:
UndefinedObject>>DoIt

and the new "continue" stack looking like this:
Generator>>fork
[] in Generator>>reset
Generator>>reset
Generator>>initializeOn:
Generator class>>on:
UndefinedObject>>DoIt ...


What I don't know is how to fix this in the general case.  This seems hard.  The debugger is executing Smalltalk (i.e. *not* stepping bytecodes) in the context of runUntilErrorOrReturnFrom: and a swapSender: could happen at an arbitrarily deep point in that execution and we want the protect blocks in runUntilErrorOrReturnFrom: to protect execution after a swapSender:.  I have no idea how to do that.

Now what's more likely is that I'm not seeing what's obviously wrong about the introduction of evaluate:onBehalfOf:.  Why does this make any difference?  It seems to me that the issue is with runUntilErrorOrReturnFrom: and swapSender:, not with evaluate:onBehalfOf:.  What am I missing?

> Best,
> Christoph

_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191230/e3e8f047/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pastedImage.png
Type: image/png
Size: 43800 bytes
Desc: pastedImage.png
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191230/e3e8f047/attachment-0003.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pastedImage.png
Type: image/png
Size: 185254 bytes
Desc: pastedImage.png
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191230/e3e8f047/attachment-0004.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pastedImage.png
Type: image/png
Size: 171961 bytes
Desc: pastedImage.png
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191230/e3e8f047/attachment-0005.png>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: bugfix swapsender (1).4.cs
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191230/e3e8f047/attachment-0001.ksh>