Hi Jaromir,

On Sun, Feb 28, 2021 at 11:08 AM Jaromir Matas <m at jaromir.net> wrote:

> Hi, the following example shows a slight inconsistency in the process
> terminantion logic:
>
>    p := Process forContext: ([] asContextWithSender: thisContext) priority:
> 40
>
>    p isTerminated   --> false
>
>    p terminate
>
>    p isTerminated   --> false
>
>
> I'm aware the example is nonsensical but I guess the terminate =>
> isTerminated logic should be followed anyway.
>
> I propose the following change in the #isTerminated condition. The idea is
> if suspendedContext is the bottomContext and pc >= endPC then it should be
> considered terminated regardless of whether there is or isn't a closure.
> The
> rest of the condition remains intact. All tests green.
>
> I also suggest a new wording of the comment (the bottomContext block
> doesn't
> necessarily need to be the block in BlockClosure>>newProcess so I removed
> the note).
>
> The suggested version:
>
> isTerminated
>         "Answer if the receiver is terminated, or at least terminating."
>         self isActiveProcess ifTrue: [^ false].
>         ^suspendedContext isNil
>           or: ["If the suspendedContext is the bottomContext and the pc is
> at the endPC,
>                 then there is nothing more to do."
>                 suspendedContext isBottomContext
>                 and: [suspendedContext pc >= suspendedContext endPC
>                                 or: [suspendedContext closure
>                                                 ifNil: [suspendedContext
> methodClass == Process
>                                                         and:
> [suspendedContext selector == #terminate]]
>                                                 ifNotNil: [false]]]]
>
>
>
I like this.  But isn't this a little bit better?

isTerminated
"Answer if the receiver is terminated, or at least terminating."
self isActiveProcess ifTrue: [^ false].
^suspendedContext isNil
 or: ["If the suspendedContext is the bottomContext and the pc is at the
endPC,
then there is nothing more to do."
suspendedContext isBottomContext
and: [suspendedContext pc >= suspendedContext endPC
or: [suspendedContext closure isNil
and: [suspendedContext methodClass == Process
and: [suspendedContext selector == #terminate]]]]]
I'm also tempted to state in a comment why being in other than a block in
Process>>#terminate implies the methods is essentially done terminating.
And there in lies the rub, to quote Shakespeare.  Would a hack like
adding a first temporary in Process>>#terminate called e.g.
terminationStatus and having Process>>terminate assign to it when
termination is essentially complete be better?

e.g.

isTerminated
"Answer if the receiver is terminated, or at least terminating."
self isActiveProcess ifTrue: [^ false].
^suspendedContext isNil
  or: ["If the suspendedContext is the bottomContext and the pc is at the
endPC,
then there is nothing more to do."
suspendedContext isBottomContext
and: [suspendedContext pc >= suspendedContext endPC
or: [suspendedContext closure isNil
and: [suspendedContext methodClass == Process
and: [suspendedContext selector == #terminate
and: [(suspendedContext localAt: 1) == #terminated]]]]]]


terminate
"Stop the process that the receiver represents forever.
Unwind to execute pending ensure:/ifCurtailed: blocks before terminating.
If the process is in the middle of a critical: critical section, release it
properly.
 N.B. terminationStatus is for the benefit of Process>>#isTerminated."

| terminationStatus ctxt unwindBlock oldList |
self isActiveProcess ifTrue:
[ctxt := thisContext.
[ctxt := ctxt findNextUnwindContextUpTo: nil.
 ctxt ~~ nil] whileTrue:
[(ctxt tempAt: 2) ifNil:
["N.B. Unlike Context>>unwindTo: we do not set complete (tempAt: 2) to
true."
unwindBlock := ctxt tempAt: 1.
thisContext terminateTo: ctxt.
unwindBlock value]].
>>> terminationStatus := #terminated.
thisContext terminateTo: nil.
self suspend.
"If the process is resumed this will provoke a cannotReturn: error.
Would self debug: thisContext title: 'Resuming a terminated process' be
better?"
^self].

"Always suspend the process first so it doesn't accidentally get woken up.
N.B. If oldList is a LinkedList then the process is runnable. If it is a
Semaphore/Mutex et al
then the process is blocked, and if it is nil then the process is already
suspended."
oldList := self suspend.
suspendedContext ifNotNil:
["Release any method marked with the <criticalSection> pragma.
 The argument is whether the process is runnable."
self releaseCriticalSection: (oldList isNil or: [oldList class ==
LinkedList]).

"If terminating a process halfways through an unwind, try to complete that
unwind block first."
(suspendedContext findNextUnwindContextUpTo: nil) ifNotNil:
[:outer|
(suspendedContext findContextSuchThat:[:c| c closure == (outer tempAt: 1)])
ifNotNil:
[:inner| "This is an unwind block currently under evaluation"
suspendedContext runUntilErrorOrReturnFrom: inner]].

>>> terminationStatus := #terminated.
ctxt := self popTo: suspendedContext bottomContext.
ctxt == suspendedContext bottomContext ifFalse:
[self debug: ctxt title: 'Unwind error during termination'].
"Set the context to its endPC for the benefit of isTerminated."
ctxt pc: ctxt endPC]

-----
> ^[^ Jaromir
> --
> Sent from: http://forum.world.st/Squeak-Dev-f45488.html
>

_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210228/245d31da/attachment.html>