[squeak-dev] The Trunk: Kernel-eem.1484.mcz

Jaromir Matas mail at jaromir.net
Mon Jun 27 23:39:21 UTC 2022


Hi Eliot,

> I think the explicit check is good, but is perhaps better if it reads self isTerminated.

Yes, checking for suspendedContext ifNil: [^self] is good as it protects against MNU (nil jump) in situations like:

p := [] newProcess suspendedContext: nil.
p signalException: Notification

Checking for self isTerminated ifTrue: [^self] would even cover your previous example:

p := [Semaphore new wait] fork.
Processor yield.
p terminate.
p signalException: Notification

In this case you can theoretically signal from a terminated process; but I guess it does no harm :)

Thanks for the examples!
best,
Jaromir


From: Eliot Miranda<mailto:eliot.miranda at gmail.com>
Sent: Monday, June 27, 2022 20:00
To: The general-purpose Squeak developers list<mailto:squeak-dev at lists.squeakfoundation.org>
Subject: Re: [squeak-dev] The Trunk: Kernel-eem.1484.mcz

Hi Jaromir,

On Mon, Jun 27, 2022 at 9:05 AM Jaromir Matas <mail at jaromir.net<mailto: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<mailto:mail at jaromir.net>


From: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org<mailto:squeak-dev-bounces at lists.squeakfoundation.org>> on behalf of Jaromir Matas <mail at jaromir.net<mailto: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<mailto: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<mailto:mail at jaromir.net>



From: Marcel Taeumel<mailto:marcel.taeumel at hpi.de>
Sent: Monday, June 27, 2022 9:38
To: squeak-dev<mailto: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<mailto: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<mailto:mail at jaromir.net>



From: Jaromir Matas<mailto:mail at jaromir.net>
Sent: Sunday, June 26, 2022 20:41
To: The general-purpose Squeak developers list<mailto: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<mailto:mail at jaromir.net>
Sent: Sunday, June 26, 2022 19:03
To: The general-purpose Squeak developers list<mailto: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<mailto:mail at jaromir.net>



From: Eliot Miranda<mailto:eliot.miranda at gmail.com>
Sent: Saturday, June 25, 2022 21:12
To: The general-purpose Squeak developers list<mailto: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<mailto: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<mailto:commits at source.squeak.org>
Sent: Saturday, June 25, 2022 1:58
To: squeak-dev at lists.squeakfoundation.org<mailto:squeak-dev at lists.squeakfoundation.org>; packages at lists.squeakfoundation.org<mailto: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/b1f685de/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 73F2E2CFA66540628CAF5F89634DC3EC.png
Type: image/png
Size: 144 bytes
Desc: 73F2E2CFA66540628CAF5F89634DC3EC.png
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220627/b1f685de/attachment-0001.png>


More information about the Squeak-dev mailing list