[squeak-dev] SIGTRAP when Proceeding from BlockCannotReturn

Jaromir Matas mail at jaromir.net
Sun May 8 15:49:37 UTC 2022


Hi Nicolas,

From: Nicolas Cellier<mailto:nicolas.cellier.aka.nice at gmail.com>
Sent: Sunday, May 8, 2022 17:26
To: The general-purpose Squeak developers list<mailto:squeak-dev at lists.squeakfoundation.org>
Subject: Re: [squeak-dev] SIGTRAP when Proceeding from BlockCannotReturn

> I have the impression that DelayWaitTimeout depends on the old primitiveSuspend (#88) behavior:

signalWaitingProcess
     "Release the given process from the semaphore it is waiting on.
     This method relies on running at highest priority so that it cannot be preempted
     by the process being released."
     beingWaitedOn := false.
     "Release the process but only if it is still waiting on its original list"
     process suspendingList == delaySemaphore ifTrue:[
         expired := true.
         process suspend; resume.
     ].
> My understanding is that this code assumes that suspending, then resuming will remove the waiting process from the list.

Yes, that’s exactly the problem; I’m enclosing the fix:

signalWaitingProcess
                "Release the given process from the semaphore it is waiting on.
                This method relies on running at highest priority so that it cannot be preempted
                by the process being released."
                | list |
                beingWaitedOn := false.
                "Release the process but only if it is still waiting on its original list"
                (list := process suspendingList) == delaySemaphore ifTrue:[
                                expired := true.
                                list remove: self ifAbsent:[].
                                process offList; resume]

Similarly, Process >> signalException: needs to be fixed too; Eliot had an elegant fix but haven’t merged it yet.

And indeed the testAtomicSuspend test depends on which primitive is used for #suspend :) So I’d leave it be as is for this release.

> So we need a suspendAndUnblock message implemented as <primitive: 88> and indeed need two different primitives.
Maybe that was the problem encountered by Eliot with virtend? Maybe there are more similar use cases in virtend...
Up to Eliot to tell.

Thanks,
Best,
Jaromir

Le dim. 8 mai 2022 à 17:17, Jaromir Matas <mail at jaromir.net<mailto:mail at jaromir.net>> a écrit :
Hi Nicolas,

>> About the latest VM, (Smalltalk processSuspensionUnblocks) answers about the behavior of primitiveSuspend (#88).
> Err,  wrong wording (Smalltalk processSuspensionUnblocks) does not describe the behavior of primitiveSuspend #(88).
> It answers whether the VM has primitives #578 and #588 (see #getCogVMFeatureFlags in VMMaker)...
>
Yes, thanks, well that was the original idea for Smalltalk processSuspensionUnblocks  to answer about the behavior of the primitiveSuspend #88; which was before the primitives #568 and #578 were introduced. (And which was when I wrote the tests using this original logic). However that approach ran into problems with some existing implementations (e.g. Virtend) and as a result the two new primitives were introduced (#568 and #578) - and apparently the logic of what Smalltalk processSuspensionUnblocks answers about changed. Since then I haven't heard from Eliot about his plans how to proceed with the new suspend semantics or whether this is it :)

In case this is still WIP I'd quite like an approach similar to Smalltalk processPreemptionYields - you'd set Smalltalk processSuspensionUnblocks true or false depending on how you want the VM to behave and the VM would use the right semantics (unless this is too naive :) ).

Regarding the tests failing with the newest VM:
testAtomicSuspend - we can remove this updated version and leave the existing one for the moment
testRevisedSuspendExpectations - we can leave this one out too
testTerminateBlockedInNestedEnsure1 - I'll take a look at these two and try to adjust the logic
testTerminateBlockedInNestedEnsure2

All remaining test in KernelTests-jar.421 work as intended.

Thanks again,
Jaromir


--

Jaromír Matas

mail at jaromir.net<mailto:mail at jaromir.net>

From: Nicolas Cellier<mailto:nicolas.cellier.aka.nice at gmail.com>
Sent: Sunday, May 8, 2022 16:22
To: The general-purpose Squeak developers list<mailto:squeak-dev at lists.squeakfoundation.org>
Subject: Re: [squeak-dev] SIGTRAP when Proceeding from BlockCannotReturn



Le dim. 8 mai 2022 à 15:59, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com<mailto:nicolas.cellier.aka.nice at gmail.com>> a écrit :


Le dim. 8 mai 2022 à 14:16, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com<mailto:nicolas.cellier.aka.nice at gmail.com>> a écrit :
Hi Jaromir

Le sam. 7 mai 2022 à 14:45, Jaromir Matas <mail at jaromir.net<mailto:mail at jaromir.net>> a écrit :
Hi Marcel, Nicolas, Eliot,


> (Nicolas) I have merged Kernel-jar.1446 which is trivial (re-signal an Exception).

Thanks!

> (Marcel) Hmm... #testSimpleResignalVsOuter1 is still failing.

In my latest trunk image it passes ok (5/7/22)

>> (Marcel) I will also take a look at KernelTests-jar.421 to check whether any new semantics are okay.
>>
> (Nicolas) Yes, I did that and got two failing tests
> testTerminateTerminatingProcess
> testResumeTerminatingProcess
>
> both failures look the same, in second self assert: terminator isSuspended.

Yes, they are supposed to fail at the moment so I suggest to make them expected failures/feature requests. However there's a stupid bug in both I’ve fixed now: they were indeed supposed to fail the following assertion:
``` self should: [terminatee terminate] raise: Error. ```

Apologies for the confusion - an updated changeset is enclosed.


However, there's another issue regarding the revised #suspend semantics Eliot has been working on. I've updated the process tests in KernelTests-jar.421 to test both the old and the new #suspend semantics. The two semantics should be distinguishable via Smalltalk processSuspensionUnblocks flag answering true in case of the old semantics and false in case of the revised one; my updated tests use this logic. However, unfortunately the latest VM answers "false" but uses the OLD suspend semantics in #suspend prim 88.

So I'm surprised you haven't observed more tests failing due to the new suspend semantics... What VM have you used – an older one? I'm on the latest trunk's VM 3183. And what is the answer of Smalltalk processSuspensionUnblocks? :) I'm utterly confused...

Hi Eliot - I may be confused here but if the current VM uses the old #suspend prim 88 semantics, shouldn't ```Smalltalk processSuspensionUnblocks``` answer true?


Indeed, I can confirm that the latest VM does answer false to (Smalltalk processSuspensionUnblocks).
Though, process suspension still unblocks as shown by the example at
https://github.com/pharo-project/pharo/issues/10669

A bit more simply, this answers false - it shouldn't:

s := Semaphore new.
p := [s wait] newProcess.
p resume.
100 milliSeconds wait.
p suspend; resume.
100 milliSeconds wait.
p isTerminated = Smalltalk processSuspensionUnblocks.

technically, this answers false - it shouldn't:

s := Semaphore new.
p := [s wait] newProcess.
p resume.
100 milliSeconds wait.
p suspend == s = Smalltalk processSuspensionUnblocks


About the latest VM, (Smalltalk processSuspensionUnblocks) answers about the behavior of primitiveSuspend (#88).
Err,  wrong wording (Smalltalk processSuspensionUnblocks) does not describe the behavior of primitiveSuspend #(88).
It answers whether the VM has primitives #578 and #588 (see #getCogVMFeatureFlags in VMMaker)...

The new Behavior is implemented by primitiveSuspendBackingUpV1 (#578) and primitiveSuspendBackingUpV2 (#588).
V1 always answers the old list (even if a Semaphore/Mutex), V2 answers nil in case of Semaphore/Mutex.

So, only ((Process>>#suspend) primitive) can give you a clue about the behavior, assuming that the primitive in question is implemented...

So it depends if we want to make it a Preference, or make the image compatible with some less capable VM...

we could implement
    Process>>primitiveSuspendAndUnblock as <primitive: 88>
    Process>>primitiveSuspend as <primitive: 588>
test the VM flag in suspend
    Process>>suspend
        ^(VMHasPrimitiveSuspendBackingUp ifNil: [VMHasPrimitiveSuspendBackingUp := Smalltalk processSuspensionUnblocks not])
            ifTrue: [self primitiveSuspend]
            ifFalse: [self primitiveSuspendAndUnblock]
Then arrange to reset VMHasPrimitiveSuspendBackingUp to nil at snapshot...

I will tell next week about the status of the VM which I used for testing, it's possibly a few months old.

> (Nicolas) I would also consider 1445 1421 and most importantly 1447.

Regarding 1421: alternatively, you might consider a newer version 1415 where the only difference is #return:from: passes a block instead of nil to #resume:through: to cause a fresh search for the first unwind context; otherwise they are equivalent.

OK, I will check again.

> (Marcel) Maybe we can just merge
>
> KernelTests-jar.393
> KernelTests-jar.418
> KernelTests-jar.421
>
> And go from there? Or are there any objections?

KernelTests-jar.393 is just a special case of a more general KernelTests-jar.418 test…

In my image all tests are green provided Smalltalk processSuspensionUnblocks answers true and the two above tests Nicolas mentioned are marked expected failures :)

Thanks very much for your feedback,
Jaromir


--

Jaromír Matas

mail at jaromir.net<mailto:mail at jaromir.net>




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220508/c0b8269f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DelayWaitTimeout-signalWaitingProcess.st
Type: application/octet-stream
Size: 625 bytes
Desc: DelayWaitTimeout-signalWaitingProcess.st
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220508/c0b8269f/attachment.obj>


More information about the Squeak-dev mailing list