[Cuis-dev] Unwind mechanism during termination is broken and inconsistent

[Juan Vuletich]

Hi Jaromir,

(inline)
On 4/30/2021 7:01 AM, Jaromir Matas via Cuis-dev wrote:
>
> Hi Juan,
>
> Awesome! I remember the idea of using the result of 
> #runUntilErrorOrReturnFrom crossed my mind for a fleeting moment… and 
> then I lost it ;) With your permission I’d like to use your fix in my 
> Squeak version as well.
>

I'm glad you like it. Yes, of course. In these low level details, it is 
a good thing to have as much common code as possible!

Besides, in general, all the code we publish for Cuis is MIT license, 
and free for any use.

> I’ve tried to rewrite your #test1ATerminate without the method calls – 
> and indeed it passes… and that’s why I missed that – it was too 
> simple; when using sends is where your fix comes to the rescue – THANKS!
>
>     | p a |
>
>     a := Array new: 4 withAll: false.
>
>     p := [
>
>              [
>
>                               [ ] ensure: [
>
>                                              [Processor activeProcess 
> suspend] ensure: [
>
>                                                            ^a at: 1 
> put: true].    "line L1"
>
>                                              a at: 2 put: 
> true]                     "line L2"
>
>                               ] ensure: [a at: 3 put: true].
>
>                               a at: 4 put: true
>
>         ] newProcess.
>
>     p resume.
>
>     Processor yield.
>
>     "make sure p is suspended and none of the unwind blocks has 
> finished yet"
>
>     self assert: p isSuspended.
>
>     a noneSatisfy: [ :b | b ].
>
>     "now terminate the process and make sure all unwind blocks have 
> finished"
>
>     p terminate.
>
>     self assert: p isTerminated.
>
>     self assert: a first & a third.
>
>     self assert: (a second | a fourth) not.
>

Yes it does. I just thought that a more real-life like test of non local 
returns should also include actual method calls!
>
> I’d like to raise a question here: I feel the second item, on line L2 
> should ideally execute too because it’s inside an unwind block halfway 
> through it’s termination. The problem is though the non-local return 
> at line L1 invokes it’s own unwind algorithm in #resume:through: which 
> ignores halfway through unwind blocks – the reason for that is 
> #resume:through: operates on the active process’s stack which makes it 
> extremely difficult to unwind halfway through blocks. I tried to apply 
> a similar tactics like in termination (control the unwind from another 
> stack) and it works well but it’s very intrusive… I may open a 
> separate discussion on that later to share the results. Do you think 
> it may be worth exploring or it’s just not worth the bother?
>

Well. This is not just in the case of process #terminate, right? To play 
with this without involving process handling, but including actual 
method calls I just tried this:

m1
     | a |
     a := Array new: 3.
     self m2: a.
     a at: 3 put: true.
     a print.

m2: a      "A"
     [1 + 2] ensure: [
         [ 3 + 4 ] ensure: [       "*1"
             true ifTrue: [
                 ^a at: 1 put: true ]]. "*2"
         a at: 2 put: true ]

In this example, the *1ensure is there only to guarantee that *2is ran, 
even if [3+4] happens to fail. If [3+4] it runs without problems, the 
result should be exactly the same as:

m2: a     "B"
     [1 + 2] ensure: [
         3 + 4.
         true ifTrue: [
             ^a at: 1 put: true ].
         a at: 2 put: true ]

Applying the same argument, the result should be the same as:

m2: a     "C"
     1 + 2.
     3 + 4.
     true ifTrue: [
         ^a at: 1 put: true ].
     a at: 2 put: true

In implementation C it is clear that a second isNil. So, the same should 
be the case for B and A.

I think that an ensured block should be guaranteed to run without 
external interference. But if it decides on its own to exit before 
running all its statements, it is it's own decision.

> Another issue: I considered using the error part of the result of 
> #runUntilErrorOrReturnFrom: to deal with situations like this (careful 
> – crashes the Cuis image without the terminate fix; with the fix it 
> works “ok”):
>
> x := nil.
>
> [self error: 'x1'] ensure: [
>
>     [self error: 'x2'] ensure: [
>
>         [self error: 'x3'] ensure: [
>
>             x:=3].
>
>         x:=2].
>
>     x:=1].
>
> x
>
> Here you have nested errors and the question is: If we abandon the 
> Debugger window, what do we want to see as a result? Without the fix 
> the image crashes badly with unwind errors, with the fix however the 
> Debugger closes without unwinding – it’s a consequence of # 
> runUntilErrorOrReturnFrom: behavior – it returns errors rather than 
> opens a debugger (and leaves the decision with the user). So what do 
> we want to see as a result – keep opening debugger windows and 
> abandoning them manually or ignoring the errors and executing the 
> assignments? That sounds like “resuming” rather than abandoning to me 
> so at the moment I don’t know and will have to think about it. I just 
> didn’t want to complicate the #terminate prematurely :)
>

I think this case is very similar to the one above.. For example, if we 
proceed the first debugger (the x1 error), the x2 debugger opens. If we 
abandon it, we are abandoning the execution of the first ensured block 
(that includes the x := 1 assignment at the end). So, no assignment is 
done. I think that the behavior of our fix is correct in this case. No 
need to simulate "resuming".

> Juan, many thanks again, I’ll study your tests and learn from them.
>

I'm happy to be of help. There's not nothing in those tests that could 
be new to you. All I did was to add nested method calls, and add the 
#resume cases, that already did work with the fix. Let me thank you. You 
did a great analysis of the issues at hand, and your fix is a great 
contribution.

I'll integrate it right now. If any further analysis provides additional 
changes, we'll integrate them too.

> Best regards,
>
> jaromir
>

Cheers,

-- 
Juan Vuletich
www.cuis-smalltalk.org
https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
https://github.com/jvuletich
https://www.linkedin.com/in/juan-vuletich-75611b3
@JuanVuletich

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20210430/c8191681/attachment.htm>