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

> Hi Nicolas,
>
>
>
> *From: *Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>
> *Sent: *Sunday, May 8, 2022 17:26
> *To: *The general-purpose Squeak developers list
> <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]
>
>
>
Maybe it's `list remove: process ifAbsent: []` instead.
Excavating offList usage would require an update of its comment...
Also, see why manipulating the list from within the image might be dangerous
http://forum.world.st/Process-gt-gt-terminate-woes-td69405.html

Similarly, Process >> signalException: needs to be fixed too; Eliot had an
> elegant fix but haven’t merged it yet.
>
>
>
Yes, I saw that too. I wonder what the behavior would be if the Process was
waiting on a Mutex/Monitor rather than a Semaphore...


> 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> 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
>
>
>
> *From: *Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>
> *Sent: *Sunday, May 8, 2022 16:22
> *To: *The general-purpose Squeak developers list
> <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> a écrit :
>
>
>
>
>
> Le dim. 8 mai 2022 à 14:16, Nicolas Cellier <
> nicolas.cellier.aka.nice at gmail.com> a écrit :
>
> Hi Jaromir
>
>
>
> Le sam. 7 mai 2022 à 14:45, Jaromir Matas <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
>
>
>
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220508/597e2b57/attachment.html>