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

Jaromir Matas mail at jaromir.net
Sun May 23 10:10:50 PDT 2021


Hi Juan,

I'm enclosing a test covering a situation where the top context itself is an unwind context. The test explores three scenarios based on the position where the execution was suspended: before entering the unwind block, right after setting complete := true but before executing the unwind block and after returning from the unwind block. #terminate should execute the unwind block in the first two cases but not in the last one.

Squeak has a convenient method Process>>#forBlock:runUntil: to step right into the desired position. So I just inlined its code into the test to make it work the same way :)

Regarding the tests demonstrating the unwind behavior in case of unhandled errors and subsequent Abandoning or Proceeding the debugger - it looks like Squeak has a machinery to do that (debugger tests) but I can't see anything similar in Cuis that would simulate opening and abandoning a debugger... Any advice?

Many thanks for all your feedback,

Best regards,

Jaromir

PS: Yesterday my message bounced back due to it’s size, I’m forwarding it again and trimming history.


From: Jaromir Matas<mailto:mail at jaromir.net>
Sent: Saturday, May 22, 2021 23:16
To: Juan Vuletich<mailto:juan at jvuletich.org>; Discussion of Cuis Smalltalk<mailto:cuis-dev at lists.cuis.st>
Subject: RE: [Cuis-dev] Unwind mechanism during termination is broken and inconsistent

Hi Juan,

Many thanks for your feedback.

I've made some final changes to the changeset:

1) extracted a repeating code to a method called Process>>complete:to: which calls #runUnwindUntilErrorOrReturnFrom: and deals with potential infinite loops caused by some errors.

2) as a result the unwind code pattern used in #terminate is almost identical to the one in #resume:through: - which I think is better for readability - when people can recognize a pattern right away :)

3) adjusted the part searching for the outer-most unwind context to catch even the top context if it's an unwind context.

4) In order to rein in the infinite loops or crashes caused by the BlockCannotReturn error I added a local variable isRecursive to BlockCannotReturn exception to be able to deal with a starting infinite loop. This is an example brought to my attention Christoph Thiede:

```
"Both statements need to be executed separately in a Workspace"
a := [true ifTrue: [^ 1] yourself]
[a value] on: BlockCannotReturn do: [:ex | ex resume]
```

With the proposed fix there's no need to change the isResumable setting of the BlockCannotReturn I suggested previously. Resuming doesn't hurt any longer :)

Regarding my suggestion to disable the Proceed button when debugging an error that happened during unwind - I'm no longer so sure it's a good idea to limit the freedom of the user in such a border situation like debugging an unwind error :) I suggest to leave it open, "use at your own risk" type of thing, for the moment.



> I think you are right in the previous email, and we'd integrate your latest change.I have already checked that none of the tests we recently added breaks because of it. If you could turn some of the arguments you provide into tests, it would be great.

I must admit I still haven't learned to build tests involving errors etc. :) But at least I updated and enclosed the list of examples that can be used as test scenarios and added some more extreme ones.

Juan, please review the enclosed changeset and let me know if you find something suspicious. Please feel free to change any names I picked or do any changes you find useful.

Best regards,

Jaromir

From: Juan Vuletich<mailto:juan at jvuletich.org>
Sent: Friday, May 21, 2021 21:08
To: Discussion of Cuis Smalltalk<mailto:cuis-dev at lists.cuis.st>
Cc: Jaromir Matas<mailto:mail at jaromir.net>; Hernan Wilkinson<mailto:hernan.wilkinson at 10pines.com>
Subject: Re: [Cuis-dev] Unwind mechanism during termination is broken and inconsistent

Hi Jaromir,

On 5/16/2021 5:26 PM, Jaromir Matas via Cuis-dev wrote:
Hi Juan,

I'm enclosing an updated changeset (please disregard the previous one); I made a stupid assumption about #runUntilErrorOrReturnFrom:. The new changeset leaves it untouched and adds its replica called #runUnwindUntilErrorOrReturnFrom: with the required functionality.

I think you are right in the previous email, and we'd integrate your latest change.I have already checked that none of the tests we recently added breaks because of it. If you could turn some of the arguments you provide into tests, it would be great.

I'd like to point out a little issue with the proposed semantics of nested errors during termination:

Below’s a simple example. If you abandon the first debugger it means you want to terminate the execution and unwind the terminated process but an error happens during the unwind. You get another debugger reporting the error and asking you what to do next - to debug the error or proceed. But proceeding a termination really means to proceed with the *unwind business only* and not to proceed with the original computation. Here's my point: Hitting Proceed will resume the original computation beyond the unwind blocks but Abandon will only finish the unwind procedure which is exactly what we want. In Squeak I simply disabled the Proceed button but in Cuis I couldn't see such a simple solution so I left it unresolved.

[self error: 'x1'] ensure: [
    [self error: 'x2'] ensure: [
        Transcript show: '2'].
    Transcript show: '1'].
Transcript show: '0'

---> you want to see '21' but proceeding the debugger will give you '210' where '0' is printed outside of unwind blocks.

So just that you know there's still a little inconsistency in the termination semantics.


I see. The issue is how to disable a button so it doesn't act? The simplest way is by saying `aButton lock: true`. We could add #enable and #disable to PluggableButtonMorph (calling self lock: ). Or perhaps call it #isEnabled: like in MenuItemMorph, although it is an ugly name. Can you prepare a change set like this? That would be great. I mean, I don't know where those calls should be made!

One more question: do you have an idea why BlockCannotReturn error is considered resumable? I'd say it's unresumable by definition…? Thanks :)

Well, that method is pretty old, and in the mean time, exception handling has gotten a lot better, thanks to several batches of improvements, latter ones by you. So, please provide a patch to change that, so it properly carries your initials.


best regards,

Jaromir

Thanks again Jaromir for this amazing work.

Cheers,


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20210523/33d021ac/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 4588-CuisCore-JaromirMatas-2021May09-20h43m-jar.003.cs.st
Type: application/octet-stream
Size: 8497 bytes
Desc: 4588-CuisCore-JaromirMatas-2021May09-20h43m-jar.003.cs.st
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20210523/33d021ac/attachment-0002.obj>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Process - terminate examples Cuis.cs
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20210523/33d021ac/attachment-0001.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testTerminateEnsureAsTopContext-BaseImageTests-jar.001.cs.st
Type: application/octet-stream
Size: 1577 bytes
Desc: testTerminateEnsureAsTopContext-BaseImageTests-jar.001.cs.st
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20210523/33d021ac/attachment-0003.obj>


More information about the Cuis-dev mailing list