Refactoring #terminate to get rid of 'cannot return' errors etc.

This is a follow up on the following threads:
[1]
http://forum.world.st/The-Inbox-Kernel-jar-1376-mcz-td5127335.html#a5127336
[2]
http://forum.world.st/The-Inbox-Kernel-jar-1377-mcz-td5127438.html#a5127439


I've noticed resuming a terminated process ends up with "cannot return" and
similar errors. I'd like to offer an alternative approach requiring just a
few tiny changes:

A process is terminated when it suspends itself or some other process does.
When someone resumes a terminated process it continues and eventually fails.
I'd like to propose a terminal method where all processes terminate:

Process>> terminated
        "When I reach this method, I'm terminated.
        Suspending or terminating me is harmless."
       
        thisContext terminateTo: nil.   "sets thisContext sender to nil"
        self suspend.
        ^thisContext restart


This approach would simplify the #isTerminated method and would require just
two little tweaks in the #terminate method:

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."

        | 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]].
                "Now all work is done and the process can terminate"
>>>> ^self terminated].

        "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]].
                ctxt := self popTo: suspendedContext bottomContext.
                ctxt == suspendedContext bottomContext ifFalse:
                        [self debug: ctxt title: 'Unwind error during
termination'].
                "Set the receiver's context to Process>>#terminated for the
benefit of
isTerminated."
>>>> ctxt setSender: nil receiver: self method: (Process>>#terminated)
arguments: {}
        ]


When the active process terminates itself it sends itself the #terminated
message parking itself in a "deathtrap". When a process is being terminated
by another process its bottom context set to #terminated, effectively
parking it in the same trap again.

That's it. Now the terminated process may get resumed or terminated again
with no error.


Now, while we're at it we could take care of the processes that terminate
"naturally" by reaching the end of their defining block. We need to refactor
#newProcess to create the terminal bottom context (the trap) and set it as
the initial sender of a newly created process for a block. If the process
finishes without explicit termination or via an exception it'll
automatically park itself in the #terminated facility:

newProcess
        "Answer a Process running the code in the receiver. The process is
not
scheduled.
        Create a new bottom context for Process>>#terminated and make it the
sender
        of the new process for the benefit of Process>>#isTerminated."
       
        | newProcess bottomContext |
        "<primitive: 19>" "Simulation guard"
        newProcess := Process new.
        bottomContext := Context sender: nil receiver: newProcess method:
(Process>>#terminated) arguments: {}.
        newProcess suspendedContext: (self asContextWithSender:
bottomContext).
        newProcess priority: Processor activePriority.
        ^newProcess

The previously used #forContext:priority: had to be inlined because of the
cross reference - the new bottom context refers to the new process and the
new process refers back to the new bottom context.

So at this point all "normal" processes (i.e. those created via newProcess)
will terminate in #terminated regardless of whether they terminated
themselves, were terminated by another party or just reached their closing
square bracket.

There's a group of potentially weird processes created via
forContext:priority: that can choose their own fate and won't in general
follow the above logic. For these cases I left the final condition in
#isTerminate: they are also considered terminated when they reach their last
instruction (and stay there):

isTerminated
        "Answer if the receiver is terminated, or at least terminating, i.e. if one
        of the following conditions is met:
        (1) the receiver is a defunct process (suspendedContext = nil or pc = nil)
        (2) the receiver is suspended within Process>>terminated, i.e. terminated
        (3) the suspendedContext is the bottomContext and the pc is at the endPC"

        self isActiveProcess ifTrue: [^false].
        ^suspendedContext isNil or: [suspendedContext isDead]
        or: [suspendedContext methodClass == Process
                and: [suspendedContext selector == #terminated]]
>>> or: [suspendedContext isBottomContext
                and: [suspendedContext atEnd]]

Examples of such processes:

p := Process forContext: [Processor activeProcess suspend] asContext
priority: Processor activePriority +1. p resume

Process forContext: [] asContext priority: 40


All tests are green but I'm aware this is just an idea that would have to be
tested thoroughly against the VM and the debugger.

Do you think this is something worth implementing though?

For me, the code is more readable, process termination more orderly, no need
for checking whether a process is terminated to avoid "cannot return"
errors, the terminated processes are easily trackable and identifiable while
debugging etc.

I'm enclosing a changeset with the proposed changes:  Refactor_#terminate.cs
<http://forum.world.st/file/t372955/Refactor_%23terminate.cs>  

Thanks,




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

Hi Jaromir,

I'm not an expert in this domain but maybe I can give my two cents anyway:

So first of all, you mentioned two motivations for your change:

1. Make terminated processes resumable
2. Clean up logic for testing termination

For 1: This is unorthodox thought because, in my view, a terminated process
has reached the end of its life cycle and does not need to be resumed again.
Can you give a concrete example where this would be useful to you? Why
wouldn't you create a new process instead in this situation? Why wouldn't
you just suspend the process? Why wouldn't you just manipulate its
suspendedContext manually? :-)

For 2: Cleaning up decades-old code is an honorable plan and might get
neglected too open in some areas. But if I understand you correctly, your
proposal would not directly allow us to remove the current #isTerminated
logic but only extend it, right? Because there could still be processes not
using the new #terminated marker and thus requiring us to keep the old
mechanisms. If this is correct, you basically propose to introduce an
alternative approach that needs to coexist with the existing one. I am not
sure whether this is really a desirable aim ... On the other hand,
#newProcess looks nicer now. Just some thoughts without a final judgment
(which I'm not qualified to pass anyway). :-)

And one small footnote:

> >>>> ctxt setSender: nil receiver: self method: (Process>>#terminated)
arguments: {}

I don't think we should consider these variables of a stack frame as mutable
fields. I'm not sure how the VM thinks about (see Eliot's description of
context marriage in [1]) and there might also be some entries in the local
stack of the context not being cleaned up. Wouldn't it be possible to
replace the context instance instead? See
Context>>#activateMethod:withArgs:receiver:class: or your own approach in
#newProcess.

Best,
Christoph

[1]
http://forum.world.st/Two-new-curious-Context-primitive-questions-tp5125779p5125783.html



-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Hi Christoph,

Many thanks for your inspiring questions! I really appreciate that.

My main motivations (apart from curiosity and learning):

1. Avoid 'cannot return' errors - by no means I want "resumable" terminated
processes ;) I just thought resuming them should be harmless; it could save
checking whether a process is terminated to avoid errors. Real life
examples? I'm thinking parallel computing where one process can terminate
while another one running in parallel might still want to resume it assuming
or hoping it is only suspended (without an explicit check that would raise
an exception). Just a thought...

Resuming a terminated process in this scenario wouldn't really "resume" it
but only suspend it again to avoid the 'cannot return' error.

2. Readability of the code (newProcess's logic isn't easy to decode)

3. Easy way to spot a terminated process while debugging :)
(suspendedContext = Process>>#terminated)

> Clean up logic for testing termination. [...] Because there could still be
> processes not using the new #terminated marker and thus requiring us to
> keep the old mechanisms.

Yes, you're right some processes might still be created the alternative way
("manually") as long as the alternative way is available but that doesn't
mean extending the current isTerminated logic - only keeping it the same. I
don't even think it can be simplified any further beyond these conditions:
        a) process is defunct / dead, or
        b) process is parked in #terminated, or
        c) process is at its last instruction

But yes, my proposal doesn't decrease complexity, only - I hope - increases
readability. I wouldn't dare to introduce anything in addition to the
existing that would increase complexity. I'm convinced the core code must be
kept at minimum complexity and maximum readability :)

> I don't think we should consider these variables of a stack frame as
> mutable fields. [...] Wouldn't it be possible to replace the context
> instance instead?

That's an interesting idea... Thanks!
Widowed, divorced or single should be ok so only married ones can have a
problem :) I can't answer this one but at least in my image it works.
However, replacing:
                ctxt setSender: nil receiver: self method: (Process>>#terminated)
arguments: {}.

with:
                ctxt := Context sender: nil receiver: self method: (Process>>#terminated)
arguments: {}

doesn't update the stack frame because nothing happens, however the
#setSender apparently must update the stack frame because it seems to work;
I guess it's because it manipulates an existing spouse (context) instead of
replacing her with a brand new. Once married the stack frames cannot replace
their spouse contexts after all ;) However I'm not an expert in these
matters. I'll explore...

Again, thanks for your feedback,
best regards,




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

Hi Christoph, all:

> your
> proposal would not directly allow us to remove the current #isTerminated
> logic but only extend it, right? Because there could still be processes
> not
> using the new #terminated marker and thus requiring us to keep the old
> mechanisms.

On second thought I think the proposal would allow us to simplify the
current #isTerminated logic. It's actually a matter of a policy: do we want
users to create processes other than the "newProcess" way? If not than we
can remove the condition checking the last instruction of the bottom
context, because newProcess no longer needs it, leaving just one condition:
is the process inside the #terminated method? (after eliminating defunct
processes). Like this:

Process >> isTerminated
        "Answer if the receiver is terminated, i.e. if the receiver is not active
and
        one of the following conditions is met:
        (1) the receiver is a defunct process (suspendedContext = nil or pc = nil)
        (2) the receiver is suspended within a method with isTerminated pragma"

        self isActiveProcess ifTrue: [^false].
        ^suspendedContext isNil or: [
                suspendedContext isDead or: [
                        (suspendedContext method pragmaAt: #isTerminated) notNil]]

I used a new pragma instead of hardcoding the class and the selector of the
terminal method:

Process >> terminated
        "When I reach this method, I'm terminated. Suspending or terminating me is
harmless."
        <isTerminated>
       
        thisContext terminateTo: nil.   "sets thisContext sender to nil"
        self suspend.
        ^thisContext restart

You can check it out in this changeset:
Refactor_#terminate_2_without_atEnd.cs
<http://forum.world.st/file/t372955/Refactor_%23terminate_2_without_atEnd.cs>  
What do you think?
best,



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

Hi Jaromir,

> Real life examples? I'm thinking parallel computing where one process can
> terminate while another one running in parallel might still want to resume
> it assuming or hoping it is only suspended (without an explicit check that
> would raise an exception). Just a thought...

Hm, in this example, you could also use a critical section to check whether
the process is terminated and then handle it respectively, couldn't you? I
still think we need a better example to conduct this discussion. Still, I
don't have great parallel programming experiences, so this is just a naive
assumption. Probably someone else can give their statement? :-)

> 2. Readability of the code (newProcess's logic isn't easy to decode)

Yes, this is probably your most important reason so far. :-)

> 3. Easy way to spot a terminated process while debugging :)
> (suspendedContext = Process>>#terminated)

You could also improve the #printString implementation of Process to reach
this goal. :-)

> However, replacing:
>                 ctxt setSender: nil receiver: self method:
> (Process>>#terminated) arguments: {}.
>
> with:
>                 ctxt := Context sender: nil receiver: self method:
> (Process>>#terminated) arguments: {}
> doesn't update the stack frame because nothing happens

After taking a second look at this part of your changeset ... Why do you
need to manipulate the stack *after* terminating the process? Wouldn't it be
possible to set the sender of the prior bottom context to the #terminated
method (provided that it does not already point to that) so that #popTo:
will automatically activate this context unless the termination was aborted?
Pseudo:

                (ctxt bottomContext method pragmaAt: #isTerminated) ifNil:
                        [ctxt bottomContext privSender: (Context sender: nil receiver:
self method: Process >> #terminated)].
arguments: {}
                ctxt := self popTo: suspendedContext bottomContext.
                (ctxt bottomContext method pragmaAt: #isTerminated) ifNil:
                        [self debug: ctxt title: 'Unwind error during termination'].

> It's actually a matter of a policy: do we want users to create processes
> other than the "newProcess" way?

That is an interesting question. When designing a new system, I would
definitively agree, just because it makes it easier to distinguish between
corrupted processes and valid terminated processes. Another aspect is
compatibility, and at the very least Process class >> #forContext:priority:
is a public instance creation method which we would need to deprecate for
public usage (with the exception of #newProcess...). The next Squeak release
will have a new major version number, but still, I think we should not
destroy bridges behind us and limit compatibility more than necessary.

That being said ... I don't know, again. Third opinions desired!

> I used a new pragma instead of hardcoding the class and the selector of
> the terminal method

I just saw that, but what's the reason for it? Pragmas to mark methods are
usually meant as extension points. So I could define any other method with
this pragma anywhere in my image and suspending a process in this method
would magically make it be displayed as terminated. Do we really need this?
Do you have any concrete example where Process >> #terminated does not
suffice? :-)

I have just another question, being completely agnostic of the relevant VM
implementation. How does the VM know when a process can be
gargabe-collected? I don't think it will invoke the image-side #isTerminated
check, so it will probably define its own logic for checking termination. If
this is the case, we should be very careful to keep both implementation in
sync to avoid dangling process instances in the image.

Best,
Christoph



-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html