[squeak-dev] Exception reraise (needs review) [was Exception reraiseFrom: (needs review)]

Jaromir Matas mail at jaromir.net
Thu Feb 2 12:22:30 UTC 2023


Hi Eliot,

I've recently discovered a bug in the Debugger causing your code for #reraise to crash the VM. Your code was ok all along, sorry for any confusion I may have caused. The Debugger fix is in Tools-jar.1189 in the Inbox.

Next, I realized you don't need a special #reraise method; you can run your scenario using #resignalAs: which will do the stitching for you. Your motivation example would look like:

| home error |
home := thisContext.
Semaphore forMutualExclusion critical:
    [[nil new]
        on: Error
        do: [:ex| error := ex copyForReraiseTo: home disableUnwinds: true]].
error resignalAs: Error

I'm enclosing a fix for Context>>resumeEvaluating: I intend to post soon; the current version of resumeEvaluating: is unnecessarily duplicating the unwind algorithm and for some reason, in your example, cuts the stack in the Debugger under the Doit context. The enclosed fix works as expected.

And lastly, I'm enclosing a correction of your #doStackCopyForReraiseTo:disableUnwinds: method; you haven't used the disableUnwindsBoolean argument.

Best,

Jaromir


--

Jaromír Matas

mail at jaromir.net


From: mail at jaromir.net<mailto:mail at jaromir.net>
Sent: Saturday, December 18, 2021 23:16
To: squeak-dev at lists.squeakfoundation.org<mailto:squeak-dev at lists.squeakfoundation.org>; eliot.miranda at gmail.com<mailto:eliot.miranda at gmail.com>
Subject: Re: [squeak-dev] Exception reraise (needs review) [was Exception reraiseFrom: (needs review)]

Hi Eliot,

I'd like to share some observations:

(1)
> Once the exception has been copied, on:do: will run unwinds and answer the error
> as the result of the [nil new] on:...do:... expression. In doing so it will run the ensure:
> block inside Semaphore>>critical: and the semaphore will get back its one excess
> signal.

I'm not sure I understand: #on:do: will certainly run unwinds but the ensure block [self signal] inside #critical: is below on:do: (meaning deeper); so my understanding is the ensure block is run normally after on:do: returns and not as an unwind during on:do:'s return. Not that it changes anything, just either the comment is incorrect or I am :) The semaphore recharges in any case and when the exception is reraised the ensure block's 'complete' variable is already set true so it won't evaluate again as desired.

> The only safe thing to do is close the debugger, or return a value to
> the sender.  But if we close the debugger unwinds will be run and we *must not*
> run unwinds twice.

Yes, closing the debugger would execute the ensure block when unwinding; I hope I understand the terminology though: unwind is the procedure during returns when the ensure blocks on the stack are identified and executed outside "normal linear flow of the computation"; otherwise, when the ensure block is executed normally it wouldn't be called unwinding, right?

(2)
I tried debugging the motivation example:

| home error |
home := thisContext.
Semaphore forMutualExclusion critical:
    [[nil new]
        on: Error
        do: [:ex| error := ex copyForReraiseTo: home disableUnwinds: true]].
error reraise

When you step over, until #reraise and then step into #reraise and in and in again you get 'endIndex is out of bounds' error (in #postCopyFrom:to:). What's worse though is when you continue debugging (stepping) then in some cases the image irreparably crashes. I can't figure out why and can't see any pattern... Sometimes it just freezes beyond CMD+dot repair, or a grey screen appears or even some weird looking grey windows with scary titles start popping-up :) The only way to get rid of it is to kill it in the OS. The described sequence (stepOver 3 times, stepIn 3 times is just the fastest way to show the error; it can be replicated many other ways).

(3)
> Maybe adding a flag to Exception and avoiding that assert when the
> exception is "in reraise mode" is better.  I don't know.

yes, I tried; it works but the crashing while debugging was even worse...

One last question: when you copy the stack fragment, you create a simple tree with the two branches sharing the same context you call 'home'. The original branch returns to 'home' and sends #reraise and the copied stack branch is then run - my question is: Does it matter that the copied branch will eventually return to a different pc position in the 'home' method than it's original version? I guess not but I'm not sure.

Thanks for sharing this code, very interesting stuff to study!
Best,



~~~
^[^    Jaromir

Sent from Squeak Inbox Talk

On 2021-12-15T11:10:49-08:00, eliot.miranda at gmail.com wrote:

> Hi All, Hi Christoph, Levente, Jaromir, Vanessa,
>
> find attached an improved implementation of copying the stack of an
> exception caught within a critical section stack for resignaling outside of
> a critical section.  The improvement and main issue here is avoiding
> running unwinds more than once.  Since the stack is copied, unwinds may be
> effectively run again if unwound in the copied stack. This implementation
> allows that to be avoided if desired.  Please review if you have time.
>
> Here's the motivating example; actually an extract of the comment
> in Exception >> copyForReraiseTo:disableUnwinds:.
>
> | home error |
> home := thisContext.
> Semaphore forMutualExclusion critical:
>     [[nil new]
>         on: Error
>         do: [:ex| error := ex copyForReraiseTo: home disableUnwinds: true]].
> error reraise
>
> Once the exception has been copied, on:do: will run unwinds and answer the
> error
> as the result of the [nil new] on:...do:... expression. In doing so it will
> run the ensure:
> block inside Semaphore>>critical: and the semaphore will get back its one
> excess
> signal.  It is therefore essential that the copy of this unwind in the
> copied exception's
> stack is not run again, otherwise the semaphore will end up with two excess
> signals.
> This is why unwinds can be disabled in the copy.
>
> It is important to understand why we disable unwinds.  Proceeding is clearly
> dangerous; we might re-enter code in a critical section, and do something
> untoward. The only safe thing to do is close the debugger, or return a
> value to
> the sender.  But if we close the debugger unwinds will be run and we *must
> not*
> run unwinds twice.
>
> On Tue, Dec 14, 2021 at 5:36 PM Eliot Miranda <eliot.miranda at gmail.com>
> wrote:
>
> > Hi All, Hi Levente, Jaromir, and Context hackers,
> >
> >     find attached a change set which supports re-raising a fatal
> > exception, caught inside a critical section, outside the critical section.
> > This is a key facility for debugging systems such as Croquet.  I expect
> > it'll also be useful in delimited continuation contexts such as Seaside.
> > The facility is closely related to Context>>copyTo: and
> > Context>>copyTo:bottomContextDo:.  The main method is
> > Exception>>copyForReraiseTo: aContext.
> >
> > This implementation is higher quality than
> > Context>>copyTo:[bottomContextDo:].  It copies the stack and it also copies
> > blocks that are created on the copied stack.  So the copied stack
> > correctly copies blocks, and hence exception handlers.
> >
> > I'm thinking of adding this to trunk.  I would like it to be reviewed
> > carefully before I do so.  Both implementation and selector names need
> > reviewing, and tests and more use cases would be nice (I have a use case in
> > Virtend/Croquet).
> >
> > Here's an example usage:
> >
> >     | sender error |
> >     sender := thisContext sender.
> >     Mutex new critical:
> >         [[nil new]
> >             on: Error
> >             do: [:ex| error := ex copyForReraiseTo: sender]].
> >     error reraiseFrom: thisContext
> >
> > A simpler usage would be
> >
> >     | home error |
> >     home := thisContext.
> >     Mutex new critical:
> >         [[nil new]
> >             on: Error
> >             do: [:ex| error := ex copyForReraiseTo: home]].
> >     error outer
> >
> > This doesn't
> > work because ProcessorScheduler>>#debugContext:title:full:contents: (et al)
> > contains
> >
> >     self assert: [thisContext hasSender: aContext]
> >
> > and hence we have to stitch the copied stack into the current stack via
> >
> >     aContext privSender: signalContext.
> >    thisContext privSender: aContext.
> >    self outer
> >
> > inside Exception>>reraiseFrom: aContext
> >
> > Maybe adding a flag to Exception and avoiding that assert when the
> > exception is "in reraise mode" is better.  I don't know.
> >
> >
> > Given that the main worker method, Context>>#copyTo:atEachStep:, is better
> > than Context>>copyTo:[bottomContextDo:]., these two could be reimplemented
> > in terms of it, e.g.:
> >
> > Context>>copyTo: aContext
> >     ^self copyTo: aContext
> >             atEachStep:
> >                 [:originalContext :copiedContext|
> >                 originalContext sender == aContext ifTrue:
> >                     [copiedContext privSender: nil]]
> >
> > Context>>copyTo: aContext bottomContextDo: aBlock
> >     ^self copyTo: aContext
> >             atEachStep:
> >                 [:originalContext :copiedContext|
> >                 originalContext sender == aContext ifTrue:
> >                     [copiedContext privSender: nil.
> >                     aBlock value: copiedContext]]
> >
> >
> > _,,,^..^,,,_
> > best, Eliot
> >
>
>
> --
> _,,,^..^,,,_
> best, Eliot
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211215/ed08069f/attachment.html>
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: reraise-methods.st
> Type: application/octet-stream
> Size: 5244 bytes
> Desc: not available
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211215/ed08069f/attachment.obj>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20230202/10357693/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Context-resumeEvaluating.st
Type: application/octet-stream
Size: 534 bytes
Desc: Context-resumeEvaluating.st
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20230202/10357693/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Exception-doStackCopyForReraiseTodisableUnwinds.st
Type: application/octet-stream
Size: 944 bytes
Desc: Exception-doStackCopyForReraiseTodisableUnwinds.st
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20230202/10357693/attachment-0001.obj>


More information about the Squeak-dev mailing list