[squeak-dev] Code reviews (was: Revisiting the fix for nested exception handling)

Thiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.de
Mon Apr 12 17:41:02 UTC 2021


Hi Nicolas,


> In order to purge the inbox, I've been in the process of reviewing brainfuck code (a very slow process). While at it, I decided to revisit the solution for nested exception handlers (see testHandlerFromAction).


<http://www.hpi.de/>
I appreciate both of these very much, thank you for your efforts! (Regular) code review and integration is an important process, in particular for a bipartite community like us. I'm aware of the fact that personally, I am only piling up new proposals that need a review but almost never have helped to carry off/review existing ones. If there is anything I (or other interested people) could do to support you in cleaning up the inbox - that goes beyond the administrative power to press the merge button (a privilege which I consider myself not mature for at the moment) -, please let me know!

Best,
Christoph

________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>
Gesendet: Sonntag, 11. April 2021 19:36 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] Revisiting the fix for nested exception handling

Hi all,
In order to purge the inbox, I've been in the process of reviewing
brainfuck code (a very slow process). While at it, I decided to
revisit the solution for nested exception handlers (see
testHandlerFromAction).

There are two similar and obsolete fixes in the inbox (due to
ContextPart->Context transition), Kernel-nice.857 and Kernel-fbs.870
(backported from Cuis) that I moved to treated inbox for that reason.
https://source.squeak.org/treated/Kernel-nice.857.diff
https://source.squeak.org/treated/Kernel-fbs.870.diff

Alas, those two solutions break the very peculiar expectations of
testHandlerReentrancy. This is a pity, because to my knowledge, those
expectations are essential for Tweak and Croquet.

There was a previous attempt from Andreas which was more a simple hack
in handleSignal:, that is to disable the handler by setting the temp
variable handlerActive in on:do: to false (self tempAt: 3 put: false).
I've generalized this solution.
https://source.squeak.org/treated/Kernel-ar.540.diff

However, there are several reasons why this does not work: the first
one, objected by Julian Fitzell here
http://lists.squeakfoundation.org/pipermail/squeak-dev/2011-January/156453.html
is that the handler must be able to handle a secondary exception
raised by the defaultAction. We can find similar expectations with
other exception handling, like resignalAs (I will publish another test
soon).

However, there is some simple strategy: before resuming the exception,
scan the context stack a second time so as to rearm the exception
handlers that we disabled during handleSignal:.

The major drawback, is that I have to spread a few self
reactivateHandlers here and there, virtually before every send of
#resumedUnchecked:.
The second drawback is that scanning the stack a second time is not
the most economical solution, but I don't care too much, we have to
make it right first.

If you find the courage to review, or more easily just to test it,
find it in the inbox...
Oups, I accidentally published in trunk...
Well the review will be forced, if you don't like it, admin please
remove or revert.
Sorry.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210412/f9d8b6d1/attachment-0001.html>


More information about the Squeak-dev mailing list