Hi Jakob,
Yes, the Behavior changed so that testHandlerFromAction can fire the
outer handler.
The handler was only deactivated during its execution, so that it
would not catch the Exception that it raises itself
(handlers are not reentrant, except if we specifically ask thru
reamHandlerDuring:)
Once the handler was finished, it was previously immediately reactivated.
That's what I changed:
- if the handler does not handle the exception, it is deactivated
- once the handler is finished it is not reactivated, except ifCurtailed:

The problem is that we have unclear expectations where we want the
handler to be active again.
My understanding is that the handlers should be reactivated
- when we resume the exception,
- when we resignalAs: a different exception, which is a specific form
of resuming
- and when performing the defaultAction.

Side note: the currently active handler will be rearmed for handling
its defaultAction, i don't even know if it should...
The specifications are quite lax.

Here, you submit another case where you want all the handlers to be rearmed.
I don't think that rearmHandlerDuring: should rearm the other
exceptions upstack.
Maybe you could try (e reactivateHandlers) instead of (e
rearmHandlerDuring: [...]).
Let's have a look at what sendNotificationsTo: does

sendNotificationsTo: aNewBlock
    self resume: (
        workBlock value: [ :barVal |
            aNewBlock value: minVal value: maxVal value: barVal
        ]
    )

We recognize the same pattern as resignalAs: and nil>>handleSignal:
resume will reactivateHandlers after the battle, that is after
evaluating aNewBlock...
I suggest this revision:

sendNotificationsTo: aNewBlock
    self reactivateHandlers; resumeUnchecked: (
        workBlock value: [ :barVal |
            aNewBlock value: minVal value: maxVal value: barVal
        ]
    )

I think that it makes rearmHandlerDuring: un-necessary.

You also raise other points.
Why the following works:
    Environment current.
    ^ self actualClassIn: Environment current
Environment current sends an extra signal, so signalling twice has a
different effect as signalling once.
You said that it's not acceptable, but I'm not so sure...

Why the debugging takes another path is another weird behavior that we
should inquire...
It ain't gonna be easy though.

Le mar. 20 avr. 2021 à 00:08, Jakob Reschke <jakres+squeak at gmail.com> a écrit :
>
> Oh by the way, this was helpful to inspect the stack in the incorrect state:
>
> MCMethodDefinition>>
> actualClass
>    (thisContext sender selector == #unload) ifTrue: [thisContext
> copyStack inspect].
>     ^ self actualClassIn: Environment current
>
> In the Inspector:
> | context |
> context := self nextHandlerContext.
> Array streamContents: [:str |
>    [context notNil] whileTrue: [str nextPut: context. context :=
> context nextHandlerContext]].
>
> | context |
> context := self.
> Array streamContents: [:str |
>    [context notNil] whileTrue: [str nextPut: context. context :=
> context sender]].
>
> Am Di., 20. Apr. 2021 um 00:02 Uhr schrieb Jakob Reschke
> <jakres+squeak at gmail.com>:
> >
> > Looks like this does no longer work as before:
> >
> > SquotImageStoreTest>>
> > suppressProgressDisplayDuring: aBlock
> >     ...
> >     aBlock
> >        on: ProgressInitiationException do: [:e |
> >           ...
> >           e rearmHandlerDuring:
> >                 [[e sendNotificationsTo: [:min :max :current | "silence"]]
> >                        on: ProgressNotification do: [:notification |
> > notification resume]]
> >    ...
> >
> > rearmHandlerDuring: does reactivate this current handler, but not
> > handlers further up the stack.
> > So e sendNotificationsTo: will evaluate the block in the package
> > loader that eventually unloads the method in the test case while all
> > handlers that are between the ProgressInitiationException signal
> > context and this ProgressInitiationException handler context on the
> > stack are deactivated, including the one that sets the correct
> > Environment (and also the source file caching, by the way). Find the
> > annotated stack below for a little more visualization.
> >
> > Previously, the handler contexts that did not fit the raised Exception
> > were not deactivated. Is the ProgressInitiationException redirection
> > concept broken in general now? Note that the ZeroDivide in
> > ProgressInitiationException>>testWith is now no longer caught.
> >
> > --- The redacted stack of my failing test with some ---annotations--->
> > [] in [] in [] in [] in [] in MCPackageLoader>>basicLoad  <--- The
> > block under 'Installing ', pkgName displayProgressFrom: ...
> > ProgressInitiationException>>sendNotificationsTo:
> > [] in [] in [] in SquotImageStoreTest>>suppressProgressDisplayDuring:
> > FullBlockClosure(BlockClosure)>>on:do:
> > [] in [] in SquotImageStoreTest>>suppressProgressDisplayDuring:
> > [] in Context>>rearmHandlerDuring:
> > FullBlockClosure(BlockClosure)>>ensure:
> > ---reactivated #1---> Context>>rearmHandlerDuring:
> > ProgressInitiationException(Exception)>>rearmHandlerDuring:
> > [] in SquotImageStoreTest>>suppressProgressDisplayDuring:
> > [] in Context>>handleSignal:
> > FullBlockClosure(BlockClosure)>>ifCurtailed:
> > Context>>handleSignal: <--- #1 sender:
> > SquotImageStoreTest>>suppressProgressDisplayDuring:
> > ...
> > Context>>handleSignal: <--- #2 sender: Environment beCurrentDuring:
> > Context>>handleSignal: <--- sender: CurrentReadOnlySourceFiles
> > class>>cacheDuring:
> > Context>>handleSignal: <--- sender: [] in [] in [] in MCPackageLoader basicLoad
> > ---triggers stack walk---> ProgressInitiationException(Exception)>>signal
> > ProgressInitiationException>>display:at:from:to:during:
> > ...
> > ByteString(String)>>displayProgressFrom:to:during:
> > [] in [] in [] in [] in MCPackageLoader>>basicLoad
> > ---deactivated---> FullBlockClosure(BlockClosure)>>on:do:
> > [] in [] in [] in MCPackageLoader>>basicLoad
> > ---deactivated---> FullBlockClosure(BlockClosure)>>on:do:
> > CurrentReadOnlySourceFiles class>>cacheDuring:
> > ...
> > MCPackageLoader>>basicLoad
> > ...
> > MCPackageLoader>>loadWithNameLike:
> > [] in SquotPackageShadow>>squotMaterializeWith:
> > ---deactivated #2---> FullBlockClosure(BlockClosure)>>on:do:
> > Environment>>beCurrentDuring:
> > SquotPackageShadow>>squotMaterializeWith:
> > ...
> > SquotImageStoreTest>>testApplyPatch
> > SquotImageStoreTest(TestCase)>>performTest
> > [] in SquotImageStoreTest>>performTest
> > ---rearmed handler #1---> FullBlockClosure(BlockClosure)>>on:do:
> > SquotImageStoreTest>>suppressProgressDisplayDuring:
> > ...
> >
> > Am Mo., 19. Apr. 2021 um 22:37 Uhr schrieb Jakob Reschke
> > <jakres+squeak at gmail.com>:
> > >
> > > Hi Nicolas,
> > >
> > > I seem to have a Heisenbug now because of this:
> > > In case you have the Git tools loaded, you should have a SquotImageStoreTest in your image. After loading this version and the follow-up fix versions of Kernel, the test SquotImageStoreTest>>testApplyPatch fails.
> > >
> > > The test fails because an initialize method is not removed from the class to be patched. When the method is about to be MCMethodDefinition>>#unload-ed, it makes a CurrentEnvironment lookup in MCMethodDefiction>>actualClass, and it returns a wrong environment. It seems that the handler to set the dynamic Environment further up in the stack is not activated for this particular lookup. Yet if I debug through the procedure, everything works as it should. If I let the test run on its own, it fails.
> > >
> > > But if I replace in MCMethodDefinition this:
> > >
> > > actualClass
> > >     ^ self actualClassIn: Environment current
> > >
> > > by this:
> > >
> > > actualClass
> > >     Environment current.
> > >     ^ self actualClassIn: Environment current
> > >
> > > Then the test is suddenly green again. I suppose that is not acceptable. ;-)
> > >
> > > It also works with Notification signal instead of the extraneous Environment current, so it seems like the signal handler stack is somehow in a wrong state, which is corrected by emitting an extraneous signal. And when I halt there to debug the situation, it also has to deal with extra signals, which corrects the state, so I cannot inspect the incorrect one. I didn't identify the reason for this behavior yet, so I am not sure whether my exception handling has bugs somewhere or whether Squeak's exception handling is bugged now.
> > >
> > > Kind regards,
> > > Jakob
> > >
> > > Am So., 11. Apr. 2021 um 19:33 Uhr schrieb <commits at source.squeak.org>:
> > >>
> > >> Nicolas Cellier uploaded a new version of Kernel to project The Trunk:
> > >> http://source.squeak.org/trunk/Kernel-nice.1384.mcz
> > >>
> > >> ==================== Summary ====================
> > >>
> > >> Name: Kernel-nice.1384
> > >> Author: nice
> > >> Time: 11 April 2021, 7:33:23.487481 pm
> > >> UUID: ecb5db19-59bc-45f0-85d3-d9296a936a68
> > >> Ancestors: Kernel-mt.1383
> > >>
> > >> Another attempt at fixing #testHandlerFromAction. Unlike previous attempts, this one preserves the expectations of #testHandlerReentrancy.
> > >>
> > >> The solution is to de-activate the handlers as we backtrack the stack, but to reactivate them before performing final exception handling actions (like resuming, resignalling or performing defaultAction). Indeed, those handlers must be able to handle a secondary exception raised in the course of this action.
> > >>
> > >> =============== Diff against Kernel-mt.1383 ===============
> > >>
> > >> Item was changed:
> > >>   ----- Method: Context>>handleSignal: (in category 'private-exceptions') -----
> > >>   handleSignal: exception
> > >>         "Sent to handler (on:do:) contexts only.  If my exception class (first arg) handles exception
> > >>          and the handler is active then execute my handle block (second arg), otherwise forward
> > >>          this message to the next handler context.  If none left, execute exception's defaultAction
> > >>          (see nil>>handleSignal:)."
> > >>
> > >>         | handlerActive val |
> > >>         "If the context has been returned from the handlerActive temp var may not be accessible."
> > >>         handlerActive := stackp >= 3 and: [(self tempAt: 3) == true].
> > >>         (((self tempAt: 1) handles: exception) and: [handlerActive]) ifFalse:
> > >> +               [stackp >= 3 ifTrue: [self tempAt: 3 put: false].
> > >> +               ^self nextHandlerContext handleSignal: exception].
> > >> -               [^self nextHandlerContext handleSignal: exception].
> > >>
> > >>         exception privHandlerContext: self contextTag.
> > >>         self tempAt: 3 put: false.  "disable self while executing handle block"
> > >>         val := [(self tempAt: 2) cull: exception]
> > >> +                       ifCurtailed: [self tempAt: 3 put: true].
> > >> -                       ensure: [self tempAt: 3 put: true].
> > >>         self return: val  "return from self if not otherwise directed in handle block"
> > >>   !
> > >>
> > >> Item was added:
> > >> + ----- Method: Context>>reactivateHandlers (in category 'private-exceptions') -----
> > >> + reactivateHandlers
> > >> +       "Private - sent to exception handler context only (on:do:).
> > >> +       Reactivate all the handlers into the chain"
> > >> +
> > >> +       self tempAt: 3 put: true. "this is temporary handlerActive in #on:do:"
> > >> +       self nextHandlerContext reactivateHandlers!
> > >>
> > >> Item was added:
> > >> + ----- Method: Exception>>reactivateHandlers (in category 'priv handling') -----
> > >> + reactivateHandlers
> > >> +       "reactivate all the exception handlers in the context chain"
> > >> +       self canSearchForSignalerContext
> > >> +               ifTrue: [signalContext nextHandlerContext reactivateHandlers]!
> > >>
> > >> Item was changed:
> > >>   ----- Method: Exception>>resignalAs: (in category 'handling') -----
> > >>   resignalAs: replacementException
> > >>         "Signal an alternative exception in place of the receiver."
> > >>
> > >> +       self reactivateHandlers.
> > >>         self resumeUnchecked: replacementException signal!
> > >>
> > >> Item was changed:
> > >>   ----- Method: UndefinedObject>>handleSignal: (in category 'bottom context') -----
> > >>   handleSignal: exception
> > >> +       "When no more handler (on:do:) context left in sender chain this gets called.  Return from signal with default action.
> > >> +       Before doing that, reactivate the handlers so that they can catch eventual secondary exceptions raised by defaultAction."
> > >> -       "When no more handler (on:do:) context left in sender chain this gets called.  Return from signal with default action."
> > >>
> > >> +       ^ exception reactivateHandlers; resumeUnchecked: exception defaultAction!
> > >> -       ^ exception resumeUnchecked: exception defaultAction!
> > >>
> > >> Item was added:
> > >> + ----- Method: UndefinedObject>>reactivateHandlers (in category 'bottom context') -----
> > >> + reactivateHandlers
> > >> +       "nothing to do for bottom context"
> > >> +
> > >> +       ^ self!
> > >>
> > >>
>