Hi Christoph,

thanks for checking this out.

> Is this really the behavior we want? Your example is of a rather theroretical nature, but if I imagine that I would start up the termination of a process but then after some time, redecide that I do not want this, then enforcing to complete the termination [...]

I have to stop you here - no, the test (or #terminate) does not enforce to complete the termination; it only enforces to complete **all unwinds** in both the terminatee and the terminator processes. The point here is if a regular termination e.g. gets suspended for whatever reason and during the suspension the terminator process gets terminated too we want to make sure all unwinds in both processes awaiting execution get executed ("all cleanups get the opportunity to complete").

> [...] in your test, the actual unwinding phase has not started yet but the process has only been suspended.

What do you mean? The real situation we're trying to simulate here is: the **termination** of the initial (aka terminatee) process has started but been suspended, i.e. a new process has been created (aka terminator) to take care of all the unwinds of the initial process. And then, before this second process managed to finish the termination of the first process, it itself became a subject to a termination. The point here is to make sure both processes which now are being terminated at the same time get properly unwound. Currently this is not the case and the first process gets simply dumped, forgotten.

> That '[] ensure: ["all the logic"]' scares me a bit. :D

Scary, huh :) But very effective and totally harmless: it's the same as #value plus guaranteed to be unwound.

> I think a better test could look like this: Start the terminator, terminate the terminator half-way (ideally from one of the unwind contexts), assert that the process is not yet terminated, start a second terminator, and then assert that the process has been terminated.

I assume the test I've proposed is a simpler version of your, more complex - or more realistic - test. I aimed simply to test the original (aka terminatee) process won't get dumped when the terminator process gets terminated before it finishes it's mission :) Other tests assert a general terminator process gets properly unwound and terminated. I tried to skip any redundant assertions covered by other tests not to further obscure this already opaque test.

> Maybe also assert that every unwind block has been evaluated exactly one time.

I noticed there are tests somewhere testing exactly this already...

> Based on such a test, we could use the #ensure: blocks in Process >> #terminate in a more cautious way.

 I'm not sure what you mean by this? If it's still reevant.

Thanks a lot, Christoph, for your comments forcing me to triplecheck my assumptions :)

Jaromir

On 2021-08-22T22:18:34+02:00, christoph.thiede at student.hpi.uni-potsdam.de wrote:

> Hi Jaromir,
> 
> short change of my mind regarding #testTerminateInTerminate: Is this really the behavior we want? Your example is of a rather theroretical nature, but if I imagine that I would start up the termination of a process but then after some time, redecide that I do not want this, then enforcing to complete the termination sounds like an unnecessary restriction to me. Especially because in your test, the actual unwinding phase has not started yet but the process has only been suspended. That '[] ensure: ["all the logic"]' scares me a bit. :D
> 
> I think a better test could look like this: Start the terminator, terminate the terminator half-way (ideally from one of the unwind contexts), assert that the process is not yet terminated, start a second terminator, and then assert that the process has been terminated. Maybe also assert that every unwind block has been evaluated exactly one time. Based on such a test, we could use the #ensure: blocks in Process >> #terminate in a more cautious way.
> 
> What do you think of this? Or which particular scenarios were you having in mind when you wrote this test? :-)
> 
> Best,
> Christoph
> 
> ---
> Sent from Squeak Inbox Talk
> 
> On 2021-05-29T02:34:13-05:00, m at jaromir.net wrote:
> 
> > Hi Christoph,
> > 
> > 
> > Christoph Thiede wrote
> > > Hi Jaromir,
> > > 
> > > thanks for writing these tests! Just a few comments here:
> > > 
> > > - #testTerminateInEnsure uses underscore assignments. This is a deprecated
> > > syntax and apparently, in current Trunk images even disabled by default.
> > > Without turning on my #allowUnderscoreAsAssignment preference, I cannot
> > > even run the test in my image. Could you please convert this to modern
> > > `:=` assignments?
> > 
> > Thanks for noticing! That's a shame ;) I must have uploaded the original
> > Cuis version I was testing (Juan brought my attention to the test). I'm
> > testing #terminate in parallel in both Squeak and Cuis to catch potential
> > irregularities (unfortunately Pharo diverged a bit too much for an "easy"
> > parallel implementation).
> > 
> > 
> > Christoph Thiede wrote
> > > - Also, there is no guarantee that in #testTerminateInEnsure, process will
> > > not have completed earlier, is it? This totally depends on the speed &
> > > implementation of the VM. We don't want this test to fail when running on
> > > a NSA machine or on your coffee machine in 2050, do we? ;P Did you
> > > consider using semaphores instead? :-)
> > 
> > This is the original Martin McClure's test, I didn't do any refinements yet
> > but I share your concerns ;)
> > 
> > Christoph Thiede wrote
> > > - #testTerminateInTerminate is very fancy. :D
> > 
> > Well, it gave me a real scare when I realized what happens when termination
> > is interrupted and terminated in the middle. Fortunately the fix is so easy
> > :)
> > 
> > Thanks very much for your comments,
> > best,
> > 
> > > Best,
> > > Christoph
> > 
> > 
> > 
> > 
> > 
> > 
> > -----
> > ^[^ Jaromir
> > --
> > Sent from: http://forum.world.st/Squeak-Dev-f45488.html
> > 
> > 
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210822/7d4894dd/attachment.html>
> 
>