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. |
Hi Nicolas,
thanks for looking into this. 4 senders and 3 implementors of #reactivateHandlers isn't too bad. However, I miss an implementation of #deactivateHandlers. I suppose it is that new line in #handleSignal:? Would you mind adding that for readability? :-) Best, Marcel
|
Hi Marcel,
yes, we can start naming the mysterious (tempAt:) and (tempAt:put:) operations with proper methods. That's what Andres Valloud did in Cuis. I did the bare minimum changes that could possibly work. That would be #deactivateHandler, because handleSignal: will only deactivate one at a time. The question about senders is whether there's one missing or not... We have to review each and every possible handling action, and ask again if ever we had a new action (not everyday's job, OK). Le lun. 12 avr. 2021 à 10:59, Marcel Taeumel <[hidden email]> a écrit : > > Hi Nicolas, > > thanks for looking into this. > > 4 senders and 3 implementors of #reactivateHandlers isn't too bad. However, I miss an implementation of #deactivateHandlers. I suppose it is that new line in #handleSignal:? Would you mind adding that for readability? :-) > > Best, > Marcel > > Am 11.04.2021 19:37:11 schrieb Nicolas Cellier <[hidden email]>: > > 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. > > |
In reply to this post by Nicolas Cellier
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). 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!
Christoph
Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
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.
Carpe Squeak!
|
Free forum by Nabble | Edit this page |