Hi Jaromir,

On Mon, Jun 27, 2022 at 9:05 AM Jaromir Matas <mail at jaromir.net> wrote:

> Hi Eliot,
>
> The attached version of #signalException fixes the stack issue (MNU) when
> the extra context returns (#jump takes care if it) - for your review.
>

Beautiful.  That's a really nice implementation.

>  I have one more question: do you prefer to continue in case of nil
> suspendedContext or would you rather return (as suggested here)?
>

Turns out it should error.  The usage that I had in VMMaker was
shutdownModule
    <doNotGenerate>
    | guiProcess activeProcess |
    threads ifNil: [^self].
    (guiProcess := self guiProcess) ~= (activeProcess := Processor
activeProcess) ifTrue:
        [guiProcess
            signalException:
                (Notification new tag: #evaluateQuit; yourself).
         Project current spawnNewProcessIfThisIsUI: activeProcess.
         activeProcess terminate].
    ...

but it is now

shutdownModule
    <doNotGenerate>
    | guiProcess activeProcess |
    threads ifNil: [^self].
    (guiProcess := self guiProcess) ~= (activeProcess := Processor
activeProcess) ifTrue:
        [guiProcess isTerminated ifFalse:
            [guiProcess
                signalException:
                    (Notification new tag: #evaluateQuit; yourself)].
         Project current spawnNewProcessIfThisIsUI: activeProcess.
         activeProcess terminate].
    ...


and the suspendedContext being nil is no longer an issue.

                …
>
>                 suspendedContext ifNil: [^self error: 'no suspended
> context!!!!'].
>
> Also, #signalException originally didn’t resume suspended processes
> (mentioned in the comments) but your latest version resumes all
> immediately;  the intended semantics is hard to judge without real
> experience, so just doublechecking. :)
>

I think the explicit check is good, but is perhaps better if it reads self
isTerminated.  Can you commit to trunk or do you need me to?

 Many thanks,
>
> Jaromir
>

You're the one who deserves thanks.

> *Jaromír Matas*
>
> mail at jaromir.net
>
>
>
> *From:* Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> on
> behalf of Jaromir Matas <mail at jaromir.net>
> *Sent:* Monday, June 27, 2022 10:12:25 AM
> *To:* The general-purpose Squeak developers list <
> squeak-dev at lists.squeakfoundation.org>
> *Subject:* Re: [squeak-dev] The Trunk: Kernel-eem.1484.mcz
>
>
>
> Hi Marcel,
>
> I think we have oversimplified #signalException.
>
> It looks like only tests use it in the Trunk (and those test are ok either
> way) but Eliot apparently has some not-so-happy usages elsewhere.
>
>
>
> My suggestion is attached; unfortunately there are no tests for this
> functionality so I guess the ultimate criteria is whether it works for
> Eliot ;)
>
> Best,
>
> Jaromir
>
> --
>
> *Jaromír Matas*
>
> mail at jaromir.net
>
>
>
> *From: *Marcel Taeumel <marcel.taeumel at hpi.de>
> *Sent: *Monday, June 27, 2022 9:38
> *To: *squeak-dev <squeak-dev at lists.squeakfoundation.org>
> *Subject: *Re: [squeak-dev] The Trunk: Kernel-eem.1484.mcz
>
>
>
> Hi Eliot, hi Jaromir --
>
>
>
> Please resolve this issue ASAP. I did not expect such a commit given that
> we are in "CODE FREEZE" at this point. A short notice/discussion would have
> been nice.
>
>
>
> Yet, thanks for working on this issue. :-) Can we leave it as is or is
> another change required?
>
>
>
> Best,
>
> Marcel
>
> Am 26.06.2022 23:57:23 schrieb Jaromir Matas <mail at jaromir.net>:
>
> Hi Eliot,
>
>
>
> How about like this:
>
> signalException: anException
>
>                 "Signal an exception in the receiver process...if the
> receiver is currently
>
>                 suspended, the exception will get signaled when the
> receiver is resumed.  If
>
>                 the receiver is blocked on a Semaphore, it will be
> immediately re-awakened
>
>                 and the exception will be signaled; if the exception is
> resumed, then the receiver
>
>                 will return to a blocked state unless the blocking
> Semaphore has excess signals"
>
>                 | oldList |
>
>                 "If we are the active process, go ahead and signal the
> exception"
>
>                 self isActiveProcess ifTrue: [^anException signal].
>
>
>
>                 "Suspend myself first to ensure that I won't run away in
> the
>
>                 midst of the following modifications."
>
>                 (oldList := myList) ifNotNil: [self suspend].
>
>
>
>                 suspendedContext ifNil: [self error: 'no suspended
> context!!'].
>
>
>
>                 "Add a new method context to the stack that will signal
> the exception"
>
>                 suspendedContext := Context
>
>                                 sender: suspendedContext
>
>                                 receiver: self
>
>                                 method: (self class lookupSelector:
> #pvtSignal:)
>
>                                 arguments: {anException}.
>
>
>
>                 "If we are not on a list to be run (i.e. this process is
> suspended), then when the
>
>                 process is resumed, it will signal the exception"
>
>
>
>                 oldList ifNotNil: [self resume]
>
>
>
>
>
> pvtSignal: anException
>
>                 "Private. This method is used to signal an exception from
> another
>
>                 process...the receiver must be the active process."
>
>
>
>                 "Since this method is not called in a normal way, we need
> to take care
>
>                 that it doesn't directly return to the caller (because I
> believe that could
>
>                 have the potential to push an unwanted object on the
> caller's stack)."
>
>
>
>                 | blocker |
>
>                 self isActiveProcess ifFalse: [^self].
>
>                 anException signal.
>
>                 blocker := Semaphore new.
>
>                 [self suspend.
>
>                 suspendedContext := suspendedContext swapSender: nil.
>
>                 self resume] fork.
>
>                 blocker wait.
>
>
>
> --
>
> *Jaromír Matas*
>
> mail at jaromir.net
>
>
>
> *From: *Jaromir Matas <mail at jaromir.net>
> *Sent: *Sunday, June 26, 2022 20:41
> *To: *The general-purpose Squeak developers list
> <squeak-dev at lists.squeakfoundation.org>
> *Subject: *Re: [squeak-dev] The Trunk: Kernel-eem.1484.mcz
>
>
>
> Hi Eliot,
>
>
>
> I think the MNU error situation corresponds to the scenario described in
> pvtSignal:list:; unless I’m mistaken or you find more elegant way I’d
> suggest to use the helper method again; fortunately nicely simplified
> thanks to the new suspend:
>
>
>
> pvtSignal: anException
>
>                 "Private. This method is used to signal an exception from
> another
>
>                 process...the receiver must be the active process."
>
>
>
>                 "Since this method is not called in a normal way, we need
> to take care
>
>                 that it doesn't directly return to the caller (because I
> believe that could
>
>                 have the potential to push an unwanted object on the
> caller's stack)."
>
>
>
>                 | blocker |
>
>                 self isActiveProcess ifFalse: [^self].
>
>                 anException signal.
>
>                 blocker := Semaphore new.
>
>                 [self suspend.
>
>                 suspendedContext := suspendedContext swapSender: nil.
>
>                 self resume] fork.
>
>                 blocker wait.
>
>
>
> What do you think?
>
> Best,
>
> Jaromir
>
>
>
>
>
> *From: *Jaromir Matas <mail at jaromir.net>
> *Sent: *Sunday, June 26, 2022 19:03
> *To: *The general-purpose Squeak developers list
> <squeak-dev at lists.squeakfoundation.org>
> *Subject: *Re: [squeak-dev] The Trunk: Kernel-eem.1484.mcz
>
>
>
> Hi Eliot,
>
>
>
> Thanks for the examples; I have a problem with this one:
>
>
>
> p := [Semaphore new wait] fork.
>
> Processor yield.
>
> p signalException: Notification
>
>
>
> It gives me MNU: UndefinedObject>>wait
>
>
>
> The reason is:
>
> 1) you place the new context (Notification signal) on top of the previous
> suspendedContext (Semaphore new wait); its pc is at the wait.
>
> 2) now when the (Notification signal) context finally returns it places
> its return value on the top of its senders stack -> and this is a problem
> because when you send the wait it takes the stack top as the receiver (but
> that’s changed in the meantime) if I understand it correctly – resulting in
> the MNU
>
>
>
> Is this a bug? I’ve never used #signalException so I’m not entirely sure
> what to expect from it but it behaves differently than in 5.3.
>
>
>
> Enclosing a screen snip. If, at that moment, you do suspendedContext pop,
> the computation will proceed as expected.
>
>
>
> I’ll keep looking at it but wanted to let you know…
>
>
>
> Thanks,
>
> Jaromir
>
>
>
> --
>
> *Jaromír Matas*
>
> mail at jaromir.net
>
>
>
> *From: *Eliot Miranda <eliot.miranda at gmail.com>
> *Sent: *Saturday, June 25, 2022 21:12
> *To: *The general-purpose Squeak developers list
> <squeak-dev at lists.squeakfoundation.org>
> *Subject: *Re: [squeak-dev] The Trunk: Kernel-eem.1484.mcz
>
>
>
>
>
>
>
> On Sat, Jun 25, 2022 at 1:50 AM Jaromir Matas <mail at jaromir.net> wrote:
>
> Hi Eliot,
>
> I missed that one, sorry.
>
> I’ve just tried this example and only got an assertion error (without your
> fix):
>
>
>
> p := [] newProcess suspendedContext: nil.
>
> p signalException: Error
>
>
>
> I just can’t figure out in which case you got the BCR exception :)
>
>
>
> In the VMMaker VM simulator, when the simulator window is closed, the
> closing code attempts to close any and all notifiers & debuggers .  I'm
> working on the threaded FFI plugin, which in the simulator has multiple
> Smalltalk processes emulating OS threads.  To close each notifier it needs
> to deliver a tagged Notification signal to the relevant process.  Some of
> these processes are waiting on semaphores.  If they are the old code would
> suspend, which would set the process's suspendedContext to nil, and result
> in a cannot return error as the signal was being delivered.
>
>
>
> So the example to test is
>
>
>
> p := [Semaphore new wait] fork.
>
> Processor yield.
>
> p signalException: Notification
>
>
>
> With the old signalException: version and the new suspend primitive this
> would crash with the BCR error.  Or something like that.  Maybe it had to
> be terminated, as in
>
>
>
> p := [Semaphore new wait] fork.
>
> Processor yield.
>
> p terminate.
>
> p signalException: Notification
>
>
>
> But you get the idea.  If the process's suspendedContext is nil then the
> context that is put on top of the stack towards the end of signalException:
> will not be able to return.
>
>
>
> Thanks!
>
> Jaromir
>
>
>
> *From: *commits at source.squeak.org
> *Sent: *Saturday, June 25, 2022 1:58
> *To: *squeak-dev at lists.squeakfoundation.org;
> packages at lists.squeakfoundation.org
> *Subject: *[squeak-dev] The Trunk: Kernel-eem.1484.mcz
>
>
>
> Eliot Miranda uploaded a new version of Kernel to project The Trunk:
> http://source.squeak.org/trunk/Kernel-eem.1484.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-eem.1484
> Author: eem
> Time: 24 June 2022, 4:58:09.258613 pm
> UUID: f8ca934f-5875-434a-8cec-4d055099bdb6
> Ancestors: Kernel-mt.1483
>
> Fix a bug in Process>>signalException: where signalling a suspended
> process would cause a cannot return exception.
>
> =============== Diff against Kernel-mt.1483 ===============
>
> Item was changed:
>   ----- Method: Process>>signalException: (in category 'signaling') -----
>   signalException: anException
>          "Signal an exception in the receiver process...if the receiver is
> currently
>          suspended, the exception will get signaled when the receiver is
> resumed.  If
>          the receiver is blocked on a Semaphore, it will be immediately
> re-awakened
>          and the exception will be signaled; if the exception is resumed,
> then the receiver
>          will return to a blocked state unless the blocking Semaphore has
> excess signals"
>
>          "If we are the active process, go ahead and signal the exception"
>           self isActiveProcess ifTrue: [^anException signal].
>
>          "Suspend myself first to ensure that I won't run away
>           in the midst of the following modifications."
> +        self isSuspended ifFalse:
> +                [self suspend].
> +        suspendedContext ifNil: [self error: 'no suspended context!!!!'].
> +        suspendedContext := Context
> -         self suspend.
> -         suspendedContext := Context
>                                                                  sender:
> suspendedContext
>                                                                  receiver:
> anException
>                                                                  method:
> (anException class lookupSelector: #signal)
>
> arguments: #().
> +        ^self resume!
> -         ^self resume!
>
>
>
>
>
>
>
>
> --
>
> _,,,^..^,,,_
>
> best, Eliot
>
>
>
>
>
>
>
>
>
>
>
>

-- 
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220627/c2e4d361/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 73F2E2CFA66540628CAF5F89634DC3EC.png
Type: image/png
Size: 144 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220627/c2e4d361/attachment.png>