The Inbox: Kernel-jar.1376.mcz

A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-jar.1376.mcz

==================== Summary ====================

Name: Kernel-jar.1376
Author: jar
Time: 28 February 2021, 7:58:42.007459 pm
UUID: d063e740-7836-a94a-9796-488741d2b2b6
Ancestors: Kernel-codefrau.1374

Fix Process >> #isTerminated inconsistent behavior - for discussion

=============== Diff against Kernel-codefrau.1374 ===============

Item was changed:
  ----- Method: Process>>isTerminated (in category 'testing') -----
  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."
-  or: ["If the suspendedContext is the bottomContext it is the block in Process>>newProcess.
-   If so, and the pc is at the endPC, the block has already sent and returned
-   from value and there is nothing more to do."
  suspendedContext isBottomContext
+ and: [suspendedContext pc >= suspendedContext endPC
+ or: [suspendedContext closure
- and: [suspendedContext closure
  ifNil: [suspendedContext methodClass == Process
  and: [suspendedContext selector == #terminate]]
+ ifNotNil: [false]]]]!
- ifNotNil: [suspendedContext pc >= suspendedContext closure endPC]]]!

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]]]]




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

Hi Eliot,

delightful! Thanks ;)


Just for my curiosity - would this be considered a bad style (or is a math
like logical expression better):


isTerminated
        "Answer if the receiver is terminated, or at least terminating.
        If the suspendedContext is the bottomContext and the pc is at the endPC,
        then there is nothing more to do.
        If an active process is terminated, it means it was suspended inside the
        #terminate method leaving pc < endPC, so it needs to be caught separately"
       
        self isActiveProcess ifTrue: [^false].
        suspendedContext ifNil: [^true].
        suspendedContext isBottomContext ifFalse: [^false].
        suspendedContext pc >= suspendedContext endPC ifTrue: [^true].
        ^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.  


My attempted explanation in the comment above but I'm not sure...


>

I tried to figure out whether you just wanted to detect termination status
as early as possible or also something else...

I guess the condition should be (not tested):

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 localAt: 1) == #terminated]]]

... because `suspendedContext closure isNil` etc. only catches terminating a
suspended process. So to also cover terminating the active process we can, I
guess,  safely remove this condition. I'm not sure though about the localAt:
1 - what if other non-terminating process had it with the same #terminated -
is it ok?


Also - #isSuspended suffers with the same problem as #isTerminated; I'll
take a look at it next.

Thanks,

Best regards









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

Hi Eliot,

To clarify some confusion in my last message I tested your idea: it works as
expected for an active process termination. In case of suspended process
termination the suspendedContext is actually not the one of
Process>>terminate so it doesn't do anything except the debugger doesn't
like it and gets stuck before sending #bottomContext following the
terminationStatus assignment (weird).

I also had to change the isTerminated condition because the context becomes
bottom one step after assigning the terminationStatus (all tests are green
but...):


isTerminated
        "Answer if the receiver is terminated, or at least terminating.
        If an active process has terminated, it means it was suspended inside the
        #terminate method and this situation needs to be caught separately.
        If the suspendedContext is the bottomContext and the pc is at the endPC,
        then there is nothing more to do."

        self isActiveProcess ifTrue: [^false].
        ^suspendedContext isNil
        or: [suspendedContext pc isNil]
        or: [suspendedContext methodClass == Process
                and: [suspendedContext selector == #terminate
                and: [(suspendedContext tempAt: 1) == #terminated]]]
        or: [suspendedContext isBottomContext
                and: [suspendedContext pc >= suspendedContext endPC]]


Thanks,



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

For consideration:

isSuspended
        "A process is suspended if it has been suspended with the suspend
primitive.
        It is distinguishable from the active process and a terminated process by
        having a non-nil suspendedContext that is either not the bottom context
        or has not reached its endPC."
        ^myList isNil
                and: [suspendedContext notNil
                and: [self isTerminated not]]

>>> new suggested comment:

        "A process is suspended if it has non-nil suspended context (i.e. is new or
        previously suspended with the suspend primitive), is not terminated and
        not waiting in a queue (Processor or Semaphore/Mutex/etc., i.e. is not
        runnable or blocked)."


The current #isSuspended suffers from the same inconsistency, i.e. answers
true even after sending terminate (in rare cases). The fix is either modify
the current condition mirroring the changes in #isTerminated or rephrase it
using #isTerminated. I'd prefer the latter but I can't foresee all
consequences. Advice welcome :)

regards,




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