Posted by Christoph Thiede on
URL: https://forum.world.st/Solving-multiple-termination-bugs-summary-proposal-tp5128285p5129805.html

Hi Jaromir,

sorry for the late reply. I'm always impressed again by your energy and
efforts for Squeak! :-) So, now let's hope that I'll be able to obtain an
overview of all the new stuff ...

First of all, please read my message in [1] first if you have not already
done it. :-) I am quite sure that halfway-executed unwind contexts should
*not* be resumed when terminating a process.

After studying your changeset, I have rewritten #terminate in the attached
changeset to 1) not resume halfway-executed contexts any longer and b) clean
up the implementation IMHO significantly. Instead of reinventing the
unwinding wheel in Process, I reused the existing logic from Context which
is important deduplication. Process-faithful debugging now also works
consistently during termination. I have also changed #testNestedUnwinding as
proposed in [1].
My implementation passes all process tests, non-local returns seem to work
as expected, and, in particular, nested debuggers are spawned and terminated
correctly for the hot example from above:

> x := nil.
> [self error: 'x1'] ensure: [
>     [self error: 'x2'] ensure: [
>         [self error: 'x3'] ensure: [
>             x:=3].
>         x:=2].
>     x:=1].

Hope you do not find any regressions. :-) I am looking forward to your
feedback!

---

Even though I have rewritten #terminate entirely, I wanted to leave you some
general comments on your approach, maybe they are helpful for your next
important contribution: :-)

- I think that the fact that you needed to skip certain exceptions manually
was a giant suboptimal hack. :-) It was a consequence of the older behavior
of #terminate to resume halfway-executed unwinded contexts. I think I have
explained in [1] why this was not a good idea.
- Instead of modifying #runUntilErrorOrReturnFrom:, I have moved the logic
to re-signal the UnhandledError into Process >> #complete:ifError:. The
reason is that roerf seems to me like something which we should change as
rarely as possible - because of its mind-blowing complexity and because it
is used in regular debugging as well. The #resumeUnchecked: part could
actually be relevant if there occurs a second UnhandledError while jumping
out of reorf.
- Some minor comments regarding coding style: I always recommend using as
many block-local temps as possible, this makes it easier to understand their
scope. In case you haven't heard it before, you might also want to google
Guard Clause. :-) It's a very helpful idiom to avoid unnecessarily indented
structures. But that's minor remarks only, of course. :-)

---

Regarding your other ideas:

> This poses a new challenge however - how to kill a debugger if we
> deliberately want or have to stop debugging a process immediately, i.e.
> without unwinding? Consider this example:
>
> `[] ensure: [self gotcha]`
>
> We'd get a debugger with a MNU error (Message Not Understood), abandon it
> and get another debugger with the same error creating an infinite
> recursion (due to how #doesNotUnderstand is written). This particular
> example is taken care of in the changeset but in general I miss a Kill
> button - has this been ever considered?

If you got an infinite recursion, that must have been another consequence of
resuming halfway-executed unwind contexts. Normally, this shouldn't happen
(and I also could not reproduce this). But yes, there might be - though very
exotic - situations in which you actually want to force-kill a process
without executing any unwind contexts. In such situations, I usually
manually set the suspendedContext's sender to nil, you can do this directly
from the debugger and it does what it should. :-) But yes, I am already
planning to add a few more secondary buttons to the debugger (they did
something similar in Pharo some time ago, too), and a small Kill button next
to Proceed/Abandon could indeed be a nice extension.

> I'm afraid my debugger implementation knowledge is presently next to none;
> I'll have to put it on my to-do list ;)

Learning by doing and debugging it. :-) If you have any questions, please
feel free to ask them on the list ...

> There's a set of basic semantics tests Tests-jar.448 in the Inbox. I
> realized I don't know how to "simulate" pressing debugger's Abandon in a
> test but I'll add more when I figure it out :) Plus more test will come if
> the change proposed here is accepted.

Great work! They're unfortunately deprecated if we do not want to resume
halfway-executed unwind contexts, but you could update them if you agree.
Apart from that, you could also take a look at DebuggerTests if you want to
learn how to write E2E tests for the debugger. But I think that your tests
already operate at a good level of abstraction.

Best,
Christoph

fix-Process-terminate.cs
<http://forum.world.st/file/t372205/fix-Process-terminate.cs> 
[1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-td5129800.html



-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html