Hi All,
here's a summary of bugs in the current termination procedure:
1. #isTerminated and #isSuspended fail to report correctly - in [1]
2. active process termination bug causing an image crash - in [2]
3. nested unwind bug: skipping some ensure blocks - in [3]
4. likely a bug: a failure to complete nested unwind blocks halfway thru execution - also in [3]
5. Christoph's discovery: a failure to correctly close the debugger in case of error or non-local return in unwind - in [4]
6. inconsistent semantics of unwinding protected blocks during active vs. suspended process termination (not reported yet - different sets of unwind blocks are executed depending on whether a process terminates itself or whether some other process initiates its termination; the proposed solution unifies active and suspended process termination and is consistent with Visual Works semantics (and goes beyond))
[1] http://forum.world.st/The-Inbox-Kernel-jar-1376-mcz-td5127335.html#a5127336 [2] http://forum.world.st/A-bug-in-active-process-termination-crashing-image-td5... [3] http://forum.world.st/Another-bug-in-Process-terminate-in-unwinding-contexts... [4] http://forum.world.st/Bug-in-Process-gt-gt-terminate-Returning-from-unwind-c...
I'd like to propose a solution - a rewrite of #terminate - addressing all of the above issues. Main points of the solution are:
(i) why differentiate active and suspended process termination? I propose to unify the two: to suspend the active process and terminate it as a suspended process. As a result the semantics/outcome of the unwind algorithm will be the same in both cases. Fixes 2 and 6.
(ii) the above leads to a susbstantial simplification of #isTerminated - a process is terminated when at the last instruction of its bottom context. Fixes 1.
(iii) unwind algorithm for incomplete unwind blocks is extended to execute the outer-most instead of the inner-most incomplete unwind context. Fixes 3 and 4.
(iv) indirect termination via #popTo is replaced with a analogue of #unwindTo: using Eliot's ingenious #runUntilErrorOrReturnFrom: the same way as in (iii). I'd very much appreciate if someone commented this part because I haven't found any clue why the indirect termination has implemented using #popTo and not #runUntilErrorOrReturnFrom. Fixes 5.
I enclose a set of examples used to compare the current Squeak semantics with the proposed one (and with VW) that can be used to build test cases.
Here's a commented code. I'll be happy to provide detailed step-by-step guidance if you find this whole idea interesting and worth implementing. I'm convinced at least parts of the proposal should be integrated as simple fixes of the current bugs. Thank you for your patience if you're still reading :) Any feedback extremely welcome. A changeset is enclosed for your convenience here: Fix_terminate_v2.cs http://forum.world.st/file/t372955/Fix_terminate_v2.cs
Process >> terminate "Stop the process that the receiver represents so that it answers true to #isTerminated. Unwind to execute pending and unfinished ensure:/ifCurtailed: blocks before terminating. If the process is in the middle of a critical: critical section, release it properly."
| ctxt unwindBlock oldList outerMost | self isActiveProcess ifTrue: [ "If terminating the active process, suspend it first and terminate it as a suspended process. Nested #terminate messages could derail the termination so let's enclose it in #ensure." [[] ensure: [self terminate]] fork. ^self suspend].
"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; if there are multiple such nested unwind blocks, try to complete the outer-most one; the inner blocks will be completed in the process." ctxt := suspendedContext. [(ctxt := ctxt findNextUnwindContextUpTo: nil) isNil] whileFalse: "Contexts under evaluation have already set their complete (tempAt: 2) to true." [(ctxt tempAt:2) ifNotNil: [outerMost := ctxt]]. outerMost ifNotNil: [ "This is the outer-most unwind context currently under evaluation; let's find an inner context executing outerMost's argument block (tempAt: 1)" (suspendedContext findContextSuchThat: [:ctx | ctx closure == (outerMost tempAt: 1)]) ifNotNil: [:inner | "Let's finish the unfinished unwind context only (i.e. up to inner) and return here" suspendedContext runUntilErrorOrReturnFrom: inner. "Update the receiver's suspendedContext (the previous step reset its sender to nil)" suspendedContext := outerMost]].
"Now all unwind blocks caught halfway through have been completed; let's execute the ones still pending. Note: #findNextUnwindContextUpTo: starts searching from the receiver's sender but the receiver itself may be an unwind context." ctxt := suspendedContext. ctxt isUnwindContext ifFalse: [ctxt := ctxt findNextUnwindContextUpTo: nil]. [ctxt isNil] whileFalse: [ (ctxt tempAt: 2) ifNil: [ ctxt tempAt: 2 put: true. unwindBlock := ctxt tempAt: 1. "Create a context for the unwind block and execute it on the unwind block's stack. Note: using #value instead of #runUntilErrorOrReturnFrom: would lead to executing the unwind on the wrong stack preventing the correct execution of non-local returns." suspendedContext := unwindBlock asContextWithSender: ctxt. suspendedContext runUntilErrorOrReturnFrom: suspendedContext]. ctxt := ctxt findNextUnwindContextUpTo: nil].
"Set the context to its endPC and its sender to nil for the benefit of isTerminated." ctxt := suspendedContext. ctxt terminateTo: nil. ctxt pc: ctxt endPC]
Process >> isTerminated "Answer if the receiver is terminated. A process is considered terminated if the suspendedContext is the bottomContext and the pc is at the endPC"
self isActiveProcess ifTrue: [^ false]. ^suspendedContext isNil or: [ suspendedContext isBottomContext and: [ suspendedContext isDead or: [suspendedContext atEnd]]]
Process >> isSuspended "A process is suspended if it has non-nil suspendedContext (e.g. new or previously suspended with the suspend primitive) and is not terminated or waiting in a scheduler or a semaphore queue (i.e. is not runnable or blocked)." ^myList isNil and: [suspendedContext notNil and: [self isTerminated not]]
Debugger >> windowIsClosing "My window is being closed; clean up. Restart the low space watcher. If the debugged process is a do-it, it may do a non-local return escaping termination so we need to insert a #terminate context under the do-it to make sure the debugged UI process will terminate."
| doItContext terminateContext | (interruptedProcess == nil or: [interruptedProcess suspendedContext isNil]) ifTrue: [^ self].
"find a do-it context; answer nil if it doesn't exist in the sender chain" doItContext := interruptedProcess suspendedContext findContextSuchThat: [:ctx | ctx methodClass = UndefinedObject and: [ ctx selector = #DoIt and: [ ctx closure isNil]]]. doItContext ifNotNil: [ "it exists so let's insert a #terminate context" terminateContext := Context sender: doItContext sender receiver: interruptedProcess method: (Process>>#terminate) arguments: {}. doItContext privSender: terminateContext ].
interruptedProcess terminate. interruptedProcess := nil. contextStack := nil. receiverInspector := nil. contextVariablesInspector := nil. Smalltalk installLowSpaceWatcher. "restart low space handler"
A few examples to illustrate the idea (many more enclosed here Some_examples_to_examine_terminate_bugs.txt http://forum.world.st/file/t372955/Some_examples_to_examine_terminate_bugs.txt ):
Ex. 1:
| p | "Suspend process inside ensure block and make sure x1 x2 and x3 are printed. Currently only x1 is printed." p := [ [ [ ] ensure: [ [ ] ensure: [ Processor activeProcess suspend. Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3'] ] fork. Processor yield. p terminate
---> x1 (instead of x1 x2 x3)
Ex. 2:
| p | Currently only x3 is printed." p := [ [ [ ] ensure: [ [ ] ensure: [ Processor activeProcess terminate. Transcript show: 'x1']. "terminate active process inside ensure block and make sure x1 x2 and x3 are printed. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3'] ] fork. Processor yield
---> x3 (instead of x1 x2 x3)
Ex. 3:
"unwind after error inside ensure block and make sure x1 x2 and x3 are printed. [ [ ] ensure: [ [ ] ensure: [ self error. Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ ^Transcript show: 'x3']
---> x3 (instead of x1 x2 x3)
Note: nested errors during unwind follow #runUntilErrorOrReturnFrom: logic, i.e. return exception and let user decide...
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi,
The enclosed changeset fixes all bugs/inconsistencies concerning process termination listed below. It's a simplified version of the previous proposal. Feedback very welcome indeed!
The idea is to simply extend the existing mechanism for completing halfway-through unwind blocks and let it deal with *all* unwind blocks.
best,
Fix_terminate_v3.cs http://forum.world.st/file/t372955/Fix_terminate_v3.cs
Jaromir Matas wrote
Hi All,
here's a summary of bugs in the current termination procedure:
#isTerminated and #isSuspended fail to report correctly - in [1]
active process termination bug causing an image crash - in [2]
nested unwind bug: skipping some ensure blocks - in [3]
likely a bug: a failure to complete nested unwind blocks halfway thru
execution - also in [3]
- Christoph's discovery: a failure to correctly close the debugger in
case of error or non-local return in unwind - in [4]
- inconsistent semantics of unwinding protected blocks during active vs.
suspended process termination
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Jaromir, many thanks for those efforts and for perseverating. It must be a bit frustrating to not receive any answer to this good work and deep investment, but there is a good reason for that: the subject is tough, and not everyone is able to give you a quick answer without reinvesting an equivalent slot of time as you personally did.
This change-set looks good to my eyes, the level of comment is good too and gives me some additional confidence. I'm probably not at your level of expertise yet, but I vote for integrating this in trunk. In the meantime, I'll leave with it in my image(s) for a few days. You can as well submit an inbox contribution as you did previously, this is the preferred way, unless the changes span many packages.
cheers
Nicolas
Le sam. 10 avr. 2021 à 10:58, Jaromir Matas m@jaromir.net a écrit :
Hi,
The enclosed changeset fixes all bugs/inconsistencies concerning process termination listed below. It's a simplified version of the previous proposal. Feedback very welcome indeed!
The idea is to simply extend the existing mechanism for completing halfway-through unwind blocks and let it deal with *all* unwind blocks.
best,
Fix_terminate_v3.cs http://forum.world.st/file/t372955/Fix_terminate_v3.cs
Jaromir Matas wrote
Hi All,
here's a summary of bugs in the current termination procedure:
#isTerminated and #isSuspended fail to report correctly - in [1]
active process termination bug causing an image crash - in [2]
nested unwind bug: skipping some ensure blocks - in [3]
likely a bug: a failure to complete nested unwind blocks halfway thru
execution - also in [3]
- Christoph's discovery: a failure to correctly close the debugger in
case of error or non-local return in unwind - in [4]
- inconsistent semantics of unwinding protected blocks during active vs.
suspended process termination
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
I'll live with it, xcuz the bad anglishe.
Le sam. 10 avr. 2021 à 23:21, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com a écrit :
Hi Jaromir, many thanks for those efforts and for perseverating. It must be a bit frustrating to not receive any answer to this good work and deep investment, but there is a good reason for that: the subject is tough, and not everyone is able to give you a quick answer without reinvesting an equivalent slot of time as you personally did.
This change-set looks good to my eyes, the level of comment is good too and gives me some additional confidence. I'm probably not at your level of expertise yet, but I vote for integrating this in trunk. In the meantime, I'll leave with it in my image(s) for a few days. You can as well submit an inbox contribution as you did previously, this is the preferred way, unless the changes span many packages.
cheers
Nicolas
Le sam. 10 avr. 2021 à 10:58, Jaromir Matas m@jaromir.net a écrit :
Hi,
The enclosed changeset fixes all bugs/inconsistencies concerning process termination listed below. It's a simplified version of the previous proposal. Feedback very welcome indeed!
The idea is to simply extend the existing mechanism for completing halfway-through unwind blocks and let it deal with *all* unwind blocks.
best,
Fix_terminate_v3.cs http://forum.world.st/file/t372955/Fix_terminate_v3.cs
Jaromir Matas wrote
Hi All,
here's a summary of bugs in the current termination procedure:
#isTerminated and #isSuspended fail to report correctly - in [1]
active process termination bug causing an image crash - in [2]
nested unwind bug: skipping some ensure blocks - in [3]
likely a bug: a failure to complete nested unwind blocks halfway thru
execution - also in [3]
- Christoph's discovery: a failure to correctly close the debugger in
case of error or non-local return in unwind - in [4]
- inconsistent semantics of unwinding protected blocks during active vs.
suspended process termination
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
BTW, I have moved your previous attempts to treated inbox, so please, do not fear to pollute the inbox again :)
Le sam. 10 avr. 2021 à 23:25, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com a écrit :
I'll live with it, xcuz the bad anglishe.
Le sam. 10 avr. 2021 à 23:21, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com a écrit :
Hi Jaromir, many thanks for those efforts and for perseverating. It must be a bit frustrating to not receive any answer to this good work and deep investment, but there is a good reason for that: the subject is tough, and not everyone is able to give you a quick answer without reinvesting an equivalent slot of time as you personally did.
This change-set looks good to my eyes, the level of comment is good too and gives me some additional confidence. I'm probably not at your level of expertise yet, but I vote for integrating this in trunk. In the meantime, I'll leave with it in my image(s) for a few days. You can as well submit an inbox contribution as you did previously, this is the preferred way, unless the changes span many packages.
cheers
Nicolas
Le sam. 10 avr. 2021 à 10:58, Jaromir Matas m@jaromir.net a écrit :
Hi,
The enclosed changeset fixes all bugs/inconsistencies concerning process termination listed below. It's a simplified version of the previous proposal. Feedback very welcome indeed!
The idea is to simply extend the existing mechanism for completing halfway-through unwind blocks and let it deal with *all* unwind blocks.
best,
Fix_terminate_v3.cs http://forum.world.st/file/t372955/Fix_terminate_v3.cs
Jaromir Matas wrote
Hi All,
here's a summary of bugs in the current termination procedure:
#isTerminated and #isSuspended fail to report correctly - in [1]
active process termination bug causing an image crash - in [2]
nested unwind bug: skipping some ensure blocks - in [3]
likely a bug: a failure to complete nested unwind blocks halfway thru
execution - also in [3]
- Christoph's discovery: a failure to correctly close the debugger in
case of error or non-local return in unwind - in [4]
- inconsistent semantics of unwinding protected blocks during active vs.
suspended process termination
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Nicolas, Thanks a lot for your encouragement! Sent to the Inbox. I'll add a set of tests a bit later.
BTW, I have moved your previous attempts to treated inbox, so please, do not fear to pollute the inbox again :)
Yes, thanks again, that's exactly what I was fearing - creating more noise in the Inbox :)
Le sam. 10 avr. 2021 à 23:21, Nicolas Cellier <[hidden email]> a écrit :
Hi Jaromir, many thanks for those efforts and for perseverating. It must be a bit frustrating to not receive any answer to this good work and deep investment, but there is a good reason for that: the subject is tough, and not everyone is able to give you a quick answer without reinvesting an equivalent slot of time as you personally did.
This change-set looks good to my eyes, the level of comment is good too and gives me some additional confidence. I'm probably not at your level of expertise yet, but I vote for integrating this in trunk. In the meantime, I'll leave with it in my image(s) for a few days. You can as well submit an inbox contribution as you did previously, this is the preferred way, unless the changes span many packages.
cheers
Nicolas
best,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Jaromir,
I am extremely sorry for not giving any feedback on all your fixes and proposals earlier. A few weeks ago, I was on holiday, then back at home, I was greeted by a huge inbox with 100+ unread emails, and somehow I just lost track of all your important updates ... Luckily, Nicolas has shown more energy than me and has helped your contributions to find their way into the Trunk! 🎉
Your summary in this post was extremely helpful for me to get an overview of all the ongoing changes. I think I understood most of it, but here are some open questions and to-dos from my point of view:
1. Regarding issue no. #5 in your list above ("Bug in Process>>#terminate | Returning from unwind contexts" [1]): Do you consider this thread resolved by now or is my answer to it still being expected? At the moment, this snippet you mentioned fails to unwind completely:
x := nil. [self error: 'x1'] ensure: [ [self error: 'x2'] ensure: [ [self error: 'x3'] ensure: [ x:=3]. x:=2]. x:=1].
2. What is the current state of this thread [2]? If all issues are resolved from your perspective, there is no need to discuss anything further - otherwise, I guess it's your turn to answer again. :)
3. I noticed that Process >> #terminate contains multiple direct sends to #runUntilErrorOrReturnFrom:. There are multiple real and potential problems with this:
3.1 Consider the following snippet:
| p | p := Processor activeProcess. Transcript showln: p == Processor activeProcess. [Transcript showln: p == Processor activeProcess] ensure: [ Transcript showln: p == Processor activeProcess]. p
Debug it, then step into the first block, and abandon the debugger. We would expect to see another "true" in the Transcript, but instead, we see a "false". This is because #runUntilErrorOrReturnFrom: does not honor process-faithful debugging. The protocol on Process, on the other hand, does so. So probably we would want to wrap these sends into #evaluate:onBehalfOf:.
3.2 As I think I mentioned somewhere else already, the result of #runUntilErrorOrReturnFrom: *must* be checked to make sure that the execution or the unwinding has not halted halfway. I don't see this in Process >> #terminate either. This might be the cause of the bug I mentioned in #1 of this post. Probably it's the best idea to discuss this in [1], too. :-)
4. What's the current state of tests? Have you contributed tests for all the issues you mentioned above? This would be awesome. :-)
Organizational notes: For sake of overview, I propose to keep this thread of the current level of abstraction and discuss all the implementation details in separate threads such as [1]. Ideally, we should also "close" all the different threads about Process issues by adding another message to each of them in order to help our future selves to keep an overview of their solutions ...
Happy processing!
Best, Christoph
[1] http://forum.world.st/Bug-in-Process-gt-gt-terminate-Returning-from-unwind-c... [2] http://forum.world.st/Refactoring-terminate-to-get-rid-of-cannot-return-erro...
----- Carpe Squeak! -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Christoph!
I apologize for not responding earlier to your great comments. I had to educate myself in error handling first :)
- Regarding issue no. #5 in your list above ("Bug in Process>>#terminate
| Returning from unwind contexts" [1]): Do you consider this thread resolved by now or is my answer to it still being expected? At the moment, this snippet you mentioned fails to unwind completely:
x := nil. [self error: 'x1'] ensure: [ [self error: 'x2'] ensure: [ [self error: 'x3'] ensure: [ x:=3]. x:=2]. x:=1].
I reread your remarks regarding how to interpret a situation like above: what do we actually abandon when new errors appear during termination and we abandon the nested debuggers? I've enclosed a changeset that makes abandoning the debuggers equivalent to terminating the debugged process (including unwind) - i.e. in the example above we'll get the first debugger, abandon it which causes a process termination, encounter the second error and start the second debugger, abandon it which again causes another termination, etc. As a result all assignments will be executed (imagine a `stream close` instead of `x:=1` so I guess it's justified).
However, because this is happening in the ensure block during unwind, it seems that abandoning is almost equivalent to proceeding :) (Not entirely though: proceed would continue after unwinding, abandon only proceeds within the unwind scope). This poses a new challenge however - how to kill a debugger if we deliberately want or have to stop debugging a process immediately, i.e. without unwinding? Consider this example:
`[] ensure: [self gotcha]`
We'd get a debugger with a MNU error (Message Not Understood), abandon it and get another debugger with the same error creating an infinite recursion (due to how #doesNotUnderstand is written). This particular example is taken care of in the changeset but in general I miss a Kill button - has this been ever considered? Note: the infinite recursion danger is present even in the current implementation but neutralized by allowing just one error during unwinding halfway through ensure blocks :)
There's also a file Kernel-jar.1403 in the Inbox: http://forum.world.st/The-Inbox-Kernel-jar-1403-mcz-td5129607.html
There are some additional changes to #terminate - mostly cleaning and simplifying the code. And more comments.
- What is the current state of this thread [2]? If all issues are
resolved from your perspective, there is no need to discuss anything further - otherwise, I guess it's your turn to answer again. :)
No progress on my side but I look forward to getting to it and responding :)
3.1 Consider the following snippet:
| p | p := Processor activeProcess. Transcript showln: p == Processor activeProcess. [Transcript showln: p == Processor activeProcess] ensure: [ Transcript showln: p == Processor activeProcess]. p
Debug it, then step into the first block, and abandon the debugger. We would expect to see another "true" in the Transcript, but instead, we see a "false". This is because #runUntilErrorOrReturnFrom: does not honor process-faithful debugging. The protocol on Process, on the other hand, does so. So probably we would want to wrap these sends into #evaluate:onBehalfOf:.
This is not a new issue; if you step further - into the ensure (i.e. the argument block) block and then abandon the debugger, you will see false instead of true even in images before the change I introduced. The reason is precisely what you described - the use of #runUntilErrorOrReturnFrom which operates on a context stack belonging to an other process and this way guarantees a correct execution of non-local returns on that process's context stack (for the price of losing process-faithful debugging).
I'm aware of the process-faithful debugging issue and I'd love to fix it, but I'm afraid my debugger implementation knowledge is presently next to none; I'll have to put it on my to-do list ;) I'd expect though simple wrapping into #evaluate:onBehalfOf: may reintroduce the original nasty non-local error bug. Would you have an idea how to wrap it so that non-local returns still worked? That would be awesome.
3.2 As I think I mentioned somewhere else already, the result of #runUntilErrorOrReturnFrom: *must* be checked to make sure that the execution or the unwinding has not halted halfway. I don't see this in Process >> #terminate either. This might be the cause of the bug I mentioned in #1 of this post. Probably it's the best idea to discuss this in [1], too. :-)
The cause of the bug in [1] (i.e. the disastrous behavior of `[self error] ensure: [^2]`) was caused by executing the non-local return (`^2`) on a wrong context stack which happened as a result of using #popTo (and consequently #evaluate:onBehalfOf:) for evaluation of the said non-local return.
Checking the return value of #runUntilErrorOrReturnFrom is the main idea behind the fix presented in this changeset. If the execution of the unwind block gets halted by an error, #runUntilErrorOrReturnFrom returns from the "foreign" stack and reports the error. Because we are operating in an unwind block I suggest the execution of the unwind block continues by opening a new debugger window for the found error, and the user will decide what to de next. To achieve this the implementation of #runUntilErrorOrReturnFrom must be modified slightly to resignal the caught exception rather than resume it - see the enclosed changeset. No other methods use #runUntilErrorOrReturnFrom so let's either accept the suggested modification or create a version of #runUntilErrorOrReturnFrom with the modified behavior.
- What's the current state of tests? Have you contributed tests for all
the issues you mentioned above? This would be awesome. :-)
There's a set of basic semantics tests Tests-jar.448 in the Inbox. I realized I don't know how to "simulate" pressing debugger's Abandon in a test but I'll add more when I figure it out :) Plus more test will come if the change proposed here is accepted.
Organizational notes: For sake of overview, I propose to keep this thread of the current level of abstraction and discuss all the implementation details in separate threads such as [1]. Ideally, we should also "close" all the different threads about Process issues by adding another message to each of them in order to help our future selves to keep an overview of their solutions ...
Absolutely, I'll go through all relevant discussions and update them.
Thanks again very much for all your comments,
best,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Sorry, forgot to enclose the changeset: Fix_terminate_v5.cs http://forum.world.st/file/t372955/Fix_terminate_v5.cs
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Christoph, hi all,
I've added a tweak to #cannotReturn: that would make sure the method won't return to the block that attempted an illegal non-local return. This would prevent crashing the image/VM when accidentally pressing Proceed or stepping over it. #terminate can take an advantage of this improvement as well - it'd be able to safely deal with illegal non-local returns and continue unwinding.
The latest version of #terminate is enclosed and in the Inbox here: http://forum.world.st/The-Inbox-Kernel-jar-1405-mcz-td5129644.html
Thanks for reviewing/commenting.
Fix_terminate_v6.cs http://forum.world.st/file/t372955/Fix_terminate_v6.cs
best,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Christoph & all,
One more update:
Christoph Thiede wrote
3.1 Consider the following snippet:
| p | p := Processor activeProcess. Transcript showln: p == Processor activeProcess. [Transcript showln: p == Processor activeProcess] ensure: [ Transcript showln: p == Processor activeProcess]. p
Debug it, then step into the first block, and abandon the debugger. We would expect to see another "true" in the Transcript, but instead, we see a "false". This is because #runUntilErrorOrReturnFrom: does not honor process-faithful debugging. The protocol on Process, on the other hand, does so. So probably we would want to wrap these sends into #evaluate:onBehalfOf:.
Yes, you were right, wrapping #runUntilErrorOrReturnFrom: into #evaluate:onBehalfOf: works well and your example now returns true as expected. Thanks very much! Here's a changeset: Fix_terminate_v7.cs http://forum.world.st/file/t372955/Fix_terminate_v7.cs
A new version in the Inbox is: http://forum.world.st/The-Inbox-Kernel-jar-1406-mcz-td5129657.html
best,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Jaromir,
sorry for the late reply. I'm always impressed again by your energy and efforts for Squeak! :-) So, now let's hope that I'll be able to obtain an overview of all the new stuff ...
First of all, please read my message in [1] first if you have not already done it. :-) I am quite sure that halfway-executed unwind contexts should *not* be resumed when terminating a process.
After studying your changeset, I have rewritten #terminate in the attached changeset to 1) not resume halfway-executed contexts any longer and b) clean up the implementation IMHO significantly. Instead of reinventing the unwinding wheel in Process, I reused the existing logic from Context which is important deduplication. Process-faithful debugging now also works consistently during termination. I have also changed #testNestedUnwinding as proposed in [1]. My implementation passes all process tests, non-local returns seem to work as expected, and, in particular, nested debuggers are spawned and terminated correctly for the hot example from above:
x := nil. [self error: 'x1'] ensure: [ [self error: 'x2'] ensure: [ [self error: 'x3'] ensure: [ x:=3]. x:=2]. x:=1].
Hope you do not find any regressions. :-) I am looking forward to your feedback!
---
Even though I have rewritten #terminate entirely, I wanted to leave you some general comments on your approach, maybe they are helpful for your next important contribution: :-)
- I think that the fact that you needed to skip certain exceptions manually was a giant suboptimal hack. :-) It was a consequence of the older behavior of #terminate to resume halfway-executed unwinded contexts. I think I have explained in [2] why this was not a good idea. - Instead of modifying #runUntilErrorOrReturnFrom:, I have moved the logic to re-signal the UnhandledError into Process >> #complete:ifError:. The reason is that roerf seems to me like something which we should change as rarely as possible - because of its mind-blowing complexity and because it is used in regular debugging as well. The #resumeUnchecked: part could actually be relevant if there occurs a second UnhandledError while jumping out of reorf. - Some minor comments regarding coding style: I always recommend using as many block-local temps as possible, this makes it easier to understand their scope. In case you haven't heard it before, you might also want to google Guard Clause. :-) It's a very helpful idiom to avoid unnecessarily indented structures. But that's minor remarks only, of course. :-)
---
Regarding your other ideas:
This poses a new challenge however - how to kill a debugger if we deliberately want or have to stop debugging a process immediately, i.e. without unwinding? Consider this example:
`[] ensure: [self gotcha]`
We'd get a debugger with a MNU error (Message Not Understood), abandon it and get another debugger with the same error creating an infinite recursion (due to how #doesNotUnderstand is written). This particular example is taken care of in the changeset but in general I miss a Kill button - has this been ever considered?
If you got an infinite recursion, that must have been another consequence of resuming halfway-executed unwind contexts. Normally, this shouldn't happen (and I also could not reproduce this). But yes, there might be - though very exotic - situations in which you actually want to force-kill a process without executing any unwind contexts. In such situations, I usually manually set the suspendedContext's sender to nil, you can do this directly from the debugger and it does what it should. :-) But yes, I am already planning to add a few more secondary buttons to the debugger (they did something similar in Pharo some time ago, too), and a small Kill button next to Proceed/Abandon could indeed be a nice extension.
I'm afraid my debugger implementation knowledge is presently next to none; I'll have to put it on my to-do list ;)
Learning by doing and debugging it. :-) If you have any questions, please feel free to ask them on the list ...
There's a set of basic semantics tests Tests-jar.448 in the Inbox. I realized I don't know how to "simulate" pressing debugger's Abandon in a test but I'll add more when I figure it out :) Plus more test will come if the change proposed here is accepted.
Great work! They're unfortunately deprecated if we do not want to resume halfway-executed unwind contexts, but you could update them if you agree. Apart from that, you could also take a look at DebuggerTests if you want to learn how to write E2E tests for the debugger. But I think that your tests already operate at a good level of abstraction.
Best, Christoph
fix-Process-terminate.cs http://forum.world.st/file/t372205/fix-Process-terminate.cs [1] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
----- Carpe Squeak! -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Christoph, hi all,
I've updated my #terminate fix to a "final" version Kernel-jar.1409 in the Inbox:
I'm still 100% convinced completing unwind block halfway-through their execution is a good idea; and it's not my idea ;)
If a process gets interrupted or is suspended in the middle of unwinding, it should be allowed to finish all unwind blocks - both halfway-through and not-yet-started. It's not necessarily a case of a raised error or abandoning the debugger - it's a general termination procedure. If we say Abandoning a debugger equals terminating the debugged process then we should terminate it including all unwinds. If we don't want to equate Abandon to general termination that doesn't mean we have to change the termination logic - we should rather change the Abandoning logic in such case :)
I think ProcessTest >> #testNestedUnwind illustrates the idea clearly:
x1 := x2 := x3 := nil. p:=[ [ [ ] ensure: [ "halfway through completion when suspended" [ ] ensure: [ "halfway through completion when suspended" Processor activeProcess suspend. "here the process gets terminated" x1 := true]. x2 := true] ] ensure: [ "not started yet when suspended" x3 := true] ] fork. Processor yield. p terminate self assert: x1 & x2 & x3.
There was nothing wrong with the process p when it got terminated in the first place and thus there's no reason to prevent it from finishing all its unwind blocks.
[...] the fact that an error has been signaled means that the signalerContext is "infected" so under no circumstances, abandoning the process should resume the execution of this infected context!
You cannot know whether an error will be raised during termination - so you should not be changing the general termination logic but rather adjusting the "debugger abandon/termination" logic; currently the consensus is: debugger Abandon equals process termination but that's not necessarily so...
Instead of reinventing the unwinding wheel in Process, I reused the existing logic from Context which is important deduplication.
Well, actually I didn't reinvent the unwind pattern but intentionally reused it with as few changes as possible - I think it improves readability because people easily recognize this pattern from #resume:, #resume:through:, #unwindTo and even the previous #terminate used the exact same pattern for an active process termination. Besides, using the same pattern for achieving a similar goal feels "safer" to me.
Instead of modifying #runUntilErrorOrReturnFrom:, I have moved the logic to re-signal the UnhandledError into Process >> #complete:ifError:. [...] The #resumeUnchecked: part could actually be relevant if there occurs a second UnhandledError while jumping out of reorf.
Yes indeed, I made a silly assumption but reverted the change back already in Kernel-jar.1408. I use its modified replica instead.
I think that the fact that you needed to skip certain exceptions manually was a giant suboptimal hack. :-)
Yes, it was a sort of my to-do list ;) I've sorted out the MessageNotUnderstood but BlockCannotReturn is still a hot topic :)
I always recommend using as many block-local temps as possible, this makes it easier to understand their scope. In case you haven't heard it before, you might also want to google Guard Clause. :-)
Again, I wanted to make as few changes as possible; but agreed absolutely :)
Thanks again very much for your comments. best regards,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi all,
there's one more "final" version (Kernel-jar.1411) of #terminate fixing unwind in a situation where #ensure is the top context when a process is terminated.
More tests covering unwind from non-local returns, unwind from nested errors and a stress test #testTerminateInEnsure presented by Martin McClure at 2019 Smalltalk conference have been added: Tests-jar.465 ToolsTests-jar.105 KernelTests-jar.405 Finally managed to add debugger tests - thanks to Marcel and Christoph for inspiration!
A patch from Kernel-jar.1410 solving catastrophic returns and infinite loops caused by BlockCannotReturn has been integrated here as well.
best,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Jaromir,
you convinced me with regard to the behavior of #terminate as well as the current definition of #testNestedUnwind - see [1]. :-) I still think my counter-proposal is relevant, we just should put it into a method with a different name. Debugger's Abandon should then use this method instead of #terminate. But please let's discuss in [1], it's already hard enough to keep an overview. :D
Regarding to your proposal:
Please see my comments in [2] about your proposed change to BlockCannotReturn.
Instead of reinventing the unwinding wheel in Process, I reused the existing logic from Context which is important deduplication.
Well, actually I didn't reinvent the unwind pattern but intentionally reused it with as few changes as possible - I think it improves readability because people easily recognize this pattern from #resume:, #resume:through:, #unwindTo and even the previous #terminate used the exact same pattern for
an active process termination. Besides, using the same pattern for achieving a similar goal feels "safer" to me.
A pattern is good, but reusing the same code is even better. :-) I still see some signification duplication between #runUntilErrorOrReturnFrom: and #runUnwindUntilErrorOrReturnFrom: as well as between Process >> #terminate and Context >> #unwindTo:. But Kernel-jar.1411 already is a good step into the right direction as far as I can tell. :-)
What remains unacceptable or dangerous to me are your hard-coded exceptions in Process >> #complete:to:. If this is crucial to prevent akwards infinite recursions, we might not be immune against similar incidents for other kinds of recursion as well. Object >> #at:, for example, is no better than Object >> #doesNotUnderstand:. Actually, any exception or exception handler might produce a similar behavior. Could you provide a few concrete examples where this check is needed? Maybe we can find a more holistic solution to this issue.
Again, I wanted to make as few changes as possible; but agreed absolutely :)
That is also a very reasonable goal which I had to learn myself the hard way. :) Keep going! :-)
Best, Christoph
[1] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri... [2] http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-tp5129706p5130114.html
Hi Christoph,
Christoph Thiede wrote
Instead of reinventing the unwinding wheel in Process, I reused the
existing logic from Context which is important deduplication. Well, actually I didn't reinvent the unwind pattern but intentionally reused it with as few changes as possible - I think it improves readability because people easily recognize this pattern from #resume:, #resume:through:, #unwindTo and even the previous #terminate used the exact same pattern for
an active process termination. Besides, using the same pattern for achieving a similar goal feels "safer" to me.
A pattern is good, but reusing the same code is even better. :-) I still see some signification duplication between #runUntilErrorOrReturnFrom: and #runUnwindUntilErrorOrReturnFrom: as well as between Process >> #terminate and Context >> #unwindTo:. But Kernel-jar.1411 already is a good step into the right direction as far as I can tell. :-)
Yes, I was wondering why I couldn't get rid of the duplication and now I think it's because there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
With regards to #unwindTo: - I haven't tested it yet but I'm wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Christoph Thiede wrote
What remains unacceptable or dangerous to me are your hard-coded exceptions in Process >> #complete:to:. If this is crucial to prevent akwards infinite recursions, we might not be immune against similar incidents for other kinds of recursion as well. Object >> #at:, for example, is no better than Object >> #doesNotUnderstand:. Actually, any exception or exception handler might produce a similar behavior. Could you provide a few concrete examples where this check is needed? Maybe we can find a more holistic solution to this issue.
Yes, this bothers me as well. I consider two common sources of infinite recursions: (1) MessageNotUnderstood - #doesNotUnderstand is intentionally written so that it resends the unknown message to facilitate writing new methods while debugging. So for the moment to recover termination from this error I suggested to deal with it on an individual basis - i.e. skip the unwind block with the error. (and yes, you're right this only applies to the "heavy" version of unwinding) (2) BlockCannonReturn - we'll discuss this in [2]
But in general - yes, any method/exception purposefully (or not) written to create a loop will break this patch (I admit it is just a patch really). I extracted it to #complete:to: to make #terminate clean; this is a WIP; I wish there was a holistic solution to this - maybe checking for exception recursion by default? :)
Christoph Thiede wrote
Again, I wanted to make as few changes as possible; but agreed absolutely :)
That is also a very reasonable goal which I had to learn myself the hard way. :) Keep going! :-)
Best, Christoph
[1] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri... [2] http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-tp5129706p5130114.html
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi All, I've sent an updated version of #teminate integrating Christoph's solution of BlockCannotReturn recursion problem (in [1]), along with a battery of tests exploring termination of nested ensure and cascading errors behavior (Debugger tests are for info and a final version can wait until releasing Christoph's proposal in [2]).
It's pretty much final, I hope...
Any complaints about #terminate - please shout ;)
[1] http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html [2] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
best,
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Jaromir Matas wrote
Hi All, I've sent an updated version of #teminate integrating Christoph's solution of BlockCannotReturn recursion problem (in [1]), along with a battery of tests exploring termination of nested ensure and cascading errors behavior (Debugger tests are for info and a final version can wait until releasing Christoph's proposal in [2]).
It's pretty much final, I hope...
Any complaints about #terminate - please shout ;)
[1] http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html [2] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
best,
Here's the link: http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
----- ^[^ Jaromir -- Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Jaromir,
Yes, I was wondering why I couldn't get rid of the duplication and now I think it's because there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
With regards to #unwindTo: - I haven't tested it yet but I'm wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in [1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
But in general - yes, any method/exception purposefully (or not) written to create a loop will break this patch (I admit it is just a patch really). I extracted it to #complete:to: to make #terminate clean; this is a WIP; I wish there was a holistic solution to this - maybe checking for exception recursion by default? :)
Sounds better already, if feasible! But how would you detect this? Unfortunately I've lost track of these infinite loops - could you maybe point me to some concrete examples that lead to an infinite recursion without this special check? :-)
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
Best, Christoph
[1] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
--- Sent from Squeak Inbox Talk
On 2021-05-31T17:01:46-05:00, m@jaromir.net wrote:
Jaromir Matas wrote
Hi All, I've sent an updated version of #teminate integrating Christoph's solution of BlockCannotReturn recursion problem (in [1]), along with a battery of tests exploring termination of nested ensure and cascading errors behavior (Debugger tests are for info and a final version can wait until releasing Christoph's proposal in [2]).
It's pretty much final, I hope...
Any complaints about #terminate - please shout ;)
[1] http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html [2] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
best,
Here's the link: http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Christoph,
Once more, apologies for taking so long to respond.
[...] there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to pack the two twin beasts into one can (I refer to "rueorf" & "ruueorf"). Also, we could consider Process >> complete:to: as a generalized version of Process >> complete and rewrite it accordingly. This is doable, maybe even desirable but for the time being I'd very much prefer to keep them separate to facilitate troubleshooting of potential problems (or localizing them). Each of the two similar methods belong to a different execution path after all. And besides, #runUntilErrorOrReturnFrom is already crazy enough and I don't feel like making it even more complex now :)
With regards to #unwindTo: - I haven't tested it yet but I'm wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in [1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw and deserves its own thread.
Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
[...]- could you maybe point me to some concrete examples that lead to an infinite recursion without this special check? :-)
Try:
``` [self error: 'error'] ensure: [self gotcha. Transcript show: 'all cleaned up! '] ```
'gotcha' represents a method unknown to the system. The code will print 'all cleaned up! ' after abandoning the error and the debugger - meaning the system will survive this code and execute all cleanups (aka unwinds). In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course because you skip completing the halfway through unwind. What I'm saying is: even if you chose the classic heavy #terminate to exit the Debugger, the code will execute remaining unwinds as one might expect (or hope for).
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected soon anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the BlockCannotReturn behavior here:
Isn't that ProceedBlockCannotReturn tautologous? I think that by actively proceeding from a BlockCannotReturn error users already accept that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of violates common sense but during our lengthy discussion you convinced me it makes a very good sense when troubleshooting :) The idea is by no means trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the process continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >> #complete:to: to deal with the new course of events.
Apart from that, the message text of your new warning is not correct if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even if someone hit Proceed without much thinking they could get an idea what's about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox: Kernel-ct.1405.mcz" thread and we can continue discussing this particular issue there. It's really a separate issue and I included your patch here along with the main termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
[...] the computation terminated is also wrong, IMO, you should get a BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be discussed thoroughly and separately in the "The Inbox: Kernel-ct.1405.mcz" thread.
If you agree I'd really appreciate if your fix could be accepted here in including the ProceedBlockCannotReturn exception I need to make it work together with the code I'm presenting here. If we come up with a better idea in the other discussion we can make amends here as well; I take it as a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's always a pleasure :) Plus - the Inbox Talk is priceless... I use it exclusively now and look forward to any improvements you might come up in the future ;)
Best,
^[^ Jaromir --
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede@student.hpi.uni-potsdam.de wrote:
Hi Jaromir,
Yes, I was wondering why I couldn't get rid of the duplication and now I think it's because there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
With regards to #unwindTo: - I haven't tested it yet but I'm wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in [1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
But in general - yes, any method/exception purposefully (or not) written to create a loop will break this patch (I admit it is just a patch really). I extracted it to #complete:to: to make #terminate clean; this is a WIP; I wish there was a holistic solution to this - maybe checking for exception recursion by default? :)
Sounds better already, if feasible! But how would you detect this? Unfortunately I've lost track of these infinite loops - could you maybe point me to some concrete examples that lead to an infinite recursion without this special check? :-)
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
Best, Christoph
[1] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
Sent from Squeak Inbox Talk
On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote:
Jaromir Matas wrote
Hi All, I've sent an updated version of #teminate integrating Christoph's solution of BlockCannotReturn recursion problem (in [1]), along with a battery of tests exploring termination of nested ensure and cascading errors behavior (Debugger tests are for info and a final version can wait until releasing Christoph's proposal in [2]).
It's pretty much final, I hope...
Any complaints about #terminate - please shout ;)
[1] http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html [2] http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
best,
Here's the link: http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to Eliot's solution of the infinite recursion in #doesNotUnderstand the #terminate code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead (because I'm not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final version of my effort to get rid of all the bugs reported in this thread (and make #terminate semantics more consistent). It passes all standard tests and the tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the 'block cannot return issue' presented in Kernel-ct.1405 (http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html).
Summary and discussion about the bugs and changes in #terminate: http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
Hi Jaromir, Hi Christoph,
On Sat, Nov 27, 2021 at 12:12 PM mail@jaromir.net wrote:
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to Eliot's solution of the infinite recursion in #doesNotUnderstand the #terminate code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead (because I'm not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final version of my effort to get rid of all the bugs reported in this thread (and make #terminate semantics more consistent). It passes all standard tests and the tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the 'block cannot return issue' presented in Kernel-ct.1405 ( http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html).
Summary and discussion about the bugs and changes in #terminate: http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
This looks good. The one thing I find concerning is running terminate in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self] newProcess. arnold resume. ^self suspend].
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self] newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend for termination'].
-- Eliot's fix of infinite recursion in doesNotUnderstand:
doesNotUnderstand: aMessage "Handle the fact that there was an attempt to send the given message to the receiver but the receiver does not understand this message (typically sent from the machine when a message is sent to the receiver and no method is defined for that selector)."
"Testing: (3 activeProcess)" | exception resumeValue | (exception := MessageNotUnderstood new) message: aMessage; receiver: self. resumeValue := exception signal. ^exception reachedDefaultHandler ifFalse: [resumeValue]
"----------> this wrapper detecting recursion is added: -------------->"
ifTrue: [ [aMessage sentTo: self] on: MessageNotUnderstood do: [:ex| | args | args := ex message arguments. (ex receiver == self and: [ex message selector ==
aMessage selector and: [(1 to: aMessage numArgs) allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) ifFalse: [ex pass]. self error: 'infinite recursion in doesNotUnderstand:']]
(see discussion http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.... )
best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-17T18:49:44+01:00, mail@jaromir.net wrote:
Hi Christoph,
Once more, apologies for taking so long to respond.
[...] there really are two distinct unwind semantics : one "light"
for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to pack the
two twin beasts into one can (I refer to "rueorf" & "ruueorf"). Also, we could consider Process >> complete:to: as a generalized version of Process
complete and rewrite it accordingly. This is doable, maybe even
desirable but for the time being I'd very much prefer to keep them separate to facilitate troubleshooting of potential problems (or localizing them). Each of the two similar methods belong to a different execution path after all. And besides, #runUntilErrorOrReturnFrom is already crazy enough and I don't feel like making it even more complex now :)
With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw and
deserves its own thread.
Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
[...]- could you maybe point me to some concrete examples that lead to
an infinite recursion without this special check? :-)
Try:
[self error: 'error'] ensure: [self gotcha. Transcript show: 'all
cleaned up! ']
'gotcha' represents a method unknown to the system. The code will print
'all cleaned up! ' after abandoning the error and the debugger - meaning the system will survive this code and execute all cleanups (aka unwinds). In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course because you skip completing the halfway through unwind. What I'm saying is: even if you chose the classic heavy #terminate to exit the Debugger, the code will execute remaining unwinds as one might expect (or hope for).
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you
maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected soon
anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the
BlockCannotReturn behavior here:
Isn't that ProceedBlockCannotReturn tautologous? I think that by
actively proceeding from a BlockCannotReturn error users already accept that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of
violates common sense but during our lengthy discussion you convinced me it makes a very good sense when troubleshooting :) The idea is by no means trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the process
continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >> #complete:to: to deal with the new course of events.
Apart from that, the message text of your new warning is not correct
if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even if
someone hit Proceed without much thinking they could get an idea what's about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox: Kernel-ct.1405.mcz"
thread and we can continue discussing this particular issue there. It's really a separate issue and I included your patch here along with the main termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
[...] the computation terminated is also wrong, IMO, you should get a
BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be
discussed thoroughly and separately in the "The Inbox: Kernel-ct.1405.mcz" thread.
If you agree I'd really appreciate if your fix could be accepted here in
including the ProceedBlockCannotReturn exception I need to make it work together with the code I'm presenting here. If we come up with a better idea in the other discussion we can make amends here as well; I take it as a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's always
a pleasure :) Plus - the Inbox Talk is priceless... I use it exclusively now and look forward to any improvements you might come up in the future ;)
Best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede at
student.hpi.uni-potsdam.de wrote:
Hi Jaromir,
Yes, I was wondering why I couldn't get rid of the duplication and
now I think it's because there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
But in general - yes, any method/exception purposefully (or not)
written to create a loop will break this patch (I admit it is just a patch really). I extracted it to #complete:to: to make #terminate clean; this is a WIP; I wish there was a holistic solution to this - maybe checking for exception recursion by default? :)
Sounds better already, if feasible! But how would you detect this?
Unfortunately I've lost track of these infinite loops - could you maybe point me to some concrete examples that lead to an infinite recursion without this special check? :-)
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you
maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
Best, Christoph
[1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
Sent from Squeak Inbox Talk
On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote:
Jaromir Matas wrote
Hi All, I've sent an updated version of #teminate integrating Christoph's
solution
of BlockCannotReturn recursion problem (in [1]), along with a
battery of
tests exploring termination of nested ensure and cascading errors
behavior
(Debugger tests are for info and a final version can wait until
releasing
Christoph's proposal in [2]).
It's pretty much final, I hope...
Any complaints about #terminate - please shout ;)
[1]
http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html
[2]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
best,
Here's the link: http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Eliot,
thanks very much for reviewing this.
This looks good. The one thing I find concerning is running terminate in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created.
Frankly, I haven't explicitly considered that... In the worst case we could revert active process termination bit like it was. Terminating an active process via a different process was just supposed to unify (and extend) the termination semantics.
At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
That's what I though I did in Process>>#complete: topContext to: aContext
[...] pair := Processor activeProcess evaluate: [topContext runUnwindUntilErrorOrReturnFrom: aContext] onBehalfOf: self. [...]
where self is the process being terminated. #runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being terminated and executes the unwind block with effective process set to the process being terminated.
Or am I mistaken?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
This is a very interesting example - thanks! Running terminate in a different process allows to relatively easily identify all unwind blocks half-way through their execution and complete them. In case setting the effective process in #complete:to: doesn't work as you requested, then I'm afraid unwinding blocks half-way through their execution would be very hard to achieve from the process being terminated itself (not impossible though - I tried but then doubted it was worth the effort). Or theoretically, it might be possible to maintain two stacks for the process being terminated - one "original" that needs to be unwound and a second "helper" one running terminate - but I haven't gotten beyond the idea, it seemed too unchartered :)
Besides, when an error happens during unwind, a debugger opens and the identity of the process running terminate may change, so the invariant may not survive the first error anyway (at least in what I was considering).
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self] newProcess. arnold resume. ^self suspend].
No, that wouldn't work because the Terminator identifies itself as the process being terminated when executing [self terminate] resulting in an infinite loop.
At the moment I can't see why setting the effective process in #complete:to: wouldn't suffice but I'll get there :) I'll try to come up with a test.
I'm intrigued: why did you use '[arnold evaluate:...' and not '[Processor activeProcess evaluate:...'; I can't figure out the difference :)
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self] newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend for termination'].
No, this wouldn't work if Processor activeProcess terminate was inside an unwind block. Terminate would attempt to proceed after self suspend and would always raise the error.
Thanks again,
Jaromir
PS: I used this example to test your suggestions:
``` | p | p := [ [ [ ] ensure: [ [Processor activeProcess terminate] ensure: [ Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3']. Transcript show: 'x4' ] newProcess. p resume. "Two yields necessary: terminate active is a two-step procedure" Processor yield. Processor yield. Transcript show: p isTerminated printString
"prints x1 x2 x3" ```
^[^ Jaromir --
Sent from Squeak Inbox Talk
On 2021-11-28T10:52:41-08:00, eliot.miranda@gmail.com wrote:
Hi Jaromir, Hi Christoph,
On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote:
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to Eliot's solution of the infinite recursion in #doesNotUnderstand the #terminate code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead (because I'm not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final version of my effort to get rid of all the bugs reported in this thread (and make #terminate semantics more consistent). It passes all standard tests and the tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the 'block cannot return issue' presented in Kernel-ct.1405 ( http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html).
Summary and discussion about the bugs and changes in #terminate: http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
This looks good. The one thing I find concerning is running terminate in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. ^self suspend].
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend for termination'].
-- Eliot's fix of infinite recursion in doesNotUnderstand:
doesNotUnderstand: aMessage "Handle the fact that there was an attempt to send the given message to the receiver but the receiver does not understand this message (typically sent from the machine when a message is sent to the receiver and no method is defined for that selector)."
"Testing: (3 activeProcess)" | exception resumeValue | (exception := MessageNotUnderstood new) message: aMessage; receiver: self. resumeValue := exception signal. ^exception reachedDefaultHandler ifFalse: [resumeValue]
"----------> this wrapper detecting recursion is added: -------------->"
ifTrue: [ [aMessage sentTo: self] on: MessageNotUnderstood do: [:ex| | args | args := ex message arguments. (ex receiver == self and: [ex message selector ==
aMessage selector and: [(1 to: aMessage numArgs) allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) ifFalse: [ex pass]. self error: 'infinite recursion in doesNotUnderstand:']]
(see discussion http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.... )
best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote:
Hi Christoph,
Once more, apologies for taking so long to respond.
[...] there really are two distinct unwind semantics : one "light"
for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to pack the
two twin beasts into one can (I refer to "rueorf" & "ruueorf"). Also, we could consider Process >> complete:to: as a generalized version of Process
complete and rewrite it accordingly. This is doable, maybe even
desirable but for the time being I'd very much prefer to keep them separate to facilitate troubleshooting of potential problems (or localizing them). Each of the two similar methods belong to a different execution path after all. And besides, #runUntilErrorOrReturnFrom is already crazy enough and I don't feel like making it even more complex now :)
With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw and
deserves its own thread.
Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
[...]- could you maybe point me to some concrete examples that lead to
an infinite recursion without this special check? :-)
Try:
[self error: 'error'] ensure: [self gotcha. Transcript show: 'all
cleaned up! ']
'gotcha' represents a method unknown to the system. The code will print
'all cleaned up! ' after abandoning the error and the debugger - meaning the system will survive this code and execute all cleanups (aka unwinds). In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course because you skip completing the halfway through unwind. What I'm saying is: even if you chose the classic heavy #terminate to exit the Debugger, the code will execute remaining unwinds as one might expect (or hope for).
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you
maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected soon
anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the
BlockCannotReturn behavior here:
Isn't that ProceedBlockCannotReturn tautologous? I think that by
actively proceeding from a BlockCannotReturn error users already accept that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of
violates common sense but during our lengthy discussion you convinced me it makes a very good sense when troubleshooting :) The idea is by no means trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the process
continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >> #complete:to: to deal with the new course of events.
Apart from that, the message text of your new warning is not correct
if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even if
someone hit Proceed without much thinking they could get an idea what's about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox: Kernel-ct.1405.mcz"
thread and we can continue discussing this particular issue there. It's really a separate issue and I included your patch here along with the main termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
[...] the computation terminated is also wrong, IMO, you should get a
BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be
discussed thoroughly and separately in the "The Inbox: Kernel-ct.1405.mcz" thread.
If you agree I'd really appreciate if your fix could be accepted here in
including the ProceedBlockCannotReturn exception I need to make it work together with the code I'm presenting here. If we come up with a better idea in the other discussion we can make amends here as well; I take it as a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's always
a pleasure :) Plus - the Inbox Talk is priceless... I use it exclusively now and look forward to any improvements you might come up in the future ;)
Best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede at
student.hpi.uni-potsdam.de wrote:
Hi Jaromir,
Yes, I was wondering why I couldn't get rid of the duplication and
now I think it's because there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
But in general - yes, any method/exception purposefully (or not)
written to create a loop will break this patch (I admit it is just a patch really). I extracted it to #complete:to: to make #terminate clean; this is a WIP; I wish there was a holistic solution to this - maybe checking for exception recursion by default? :)
Sounds better already, if feasible! But how would you detect this?
Unfortunately I've lost track of these infinite loops - could you maybe point me to some concrete examples that lead to an infinite recursion without this special check? :-)
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you
maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
Best, Christoph
[1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
Sent from Squeak Inbox Talk
On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote:
Jaromir Matas wrote
Hi All, I've sent an updated version of #teminate integrating Christoph's
solution
of BlockCannotReturn recursion problem (in [1]), along with a
battery of
tests exploring termination of nested ensure and cascading errors
behavior
(Debugger tests are for info and a final version can wait until
releasing
Christoph's proposal in [2]).
It's pretty much final, I hope...
Any complaints about #terminate - please shout ;)
[1]
http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html
[2]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
best,
Here's the link: http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
Hi Eliot, all,
This looks good. The one thing I find concerning is running terminate in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess ifAbsent: []]
Would this test prove #terminate works as you'd expect? ``` | q | [ [q := Processor activeProcess. Processor activeProcess suspend] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield. q terminate ``` Or with a raised exception: ``` | q | [ [q := Processor activeProcess. self error] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield. ``` (If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, indeed)
Please let me know if this is acceptable.
Thanks again,
^[^ Jaromir --
Sent from Squeak Inbox Talk
On 2021-11-28T22:57:25+01:00, mail@jaromir.net wrote:
Hi Eliot,
thanks very much for reviewing this.
This looks good. The one thing I find concerning is running terminate in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created.
Frankly, I haven't explicitly considered that... In the worst case we could revert active process termination bit like it was. Terminating an active process via a different process was just supposed to unify (and extend) the termination semantics.
At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
That's what I though I did in Process>>#complete: topContext to: aContext
[...] pair := Processor activeProcess evaluate: [topContext runUnwindUntilErrorOrReturnFrom: aContext] onBehalfOf: self. [...]
where self is the process being terminated. #runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being terminated and executes the unwind block with effective process set to the process being terminated.
Or am I mistaken?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
This is a very interesting example - thanks! Running terminate in a different process allows to relatively easily identify all unwind blocks half-way through their execution and complete them. In case setting the effective process in #complete:to: doesn't work as you requested, then I'm afraid unwinding blocks half-way through their execution would be very hard to achieve from the process being terminated itself (not impossible though - I tried but then doubted it was worth the effort). Or theoretically, it might be possible to maintain two stacks for the process being terminated - one "original" that needs to be unwound and a second "helper" one running terminate - but I haven't gotten beyond the idea, it seemed too unchartered :)
Besides, when an error happens during unwind, a debugger opens and the identity of the process running terminate may change, so the invariant may not survive the first error anyway (at least in what I was considering).
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self] newProcess. arnold resume. ^self suspend].
No, that wouldn't work because the Terminator identifies itself as the process being terminated when executing [self terminate] resulting in an infinite loop.
At the moment I can't see why setting the effective process in #complete:to: wouldn't suffice but I'll get there :) I'll try to come up with a test.
I'm intrigued: why did you use '[arnold evaluate:...' and not '[Processor activeProcess evaluate:...'; I can't figure out the difference :)
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self] newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend for termination'].
No, this wouldn't work if Processor activeProcess terminate was inside an unwind block. Terminate would attempt to proceed after self suspend and would always raise the error.
Thanks again,
Jaromir
PS: I used this example to test your suggestions:
| p | p := [ [ [ ] ensure: [ [Processor activeProcess terminate] ensure: [ Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3']. Transcript show: 'x4' ] newProcess. p resume. "Two yields necessary: terminate active is a two-step procedure" Processor yield. Processor yield. Transcript show: p isTerminated printString "prints x1 x2 x3"
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote:
Hi Jaromir, Hi Christoph,
On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote:
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to Eliot's solution of the infinite recursion in #doesNotUnderstand the #terminate code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead (because I'm not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final version of my effort to get rid of all the bugs reported in this thread (and make #terminate semantics more consistent). It passes all standard tests and the tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the 'block cannot return issue' presented in Kernel-ct.1405 ( http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html).
Summary and discussion about the bugs and changes in #terminate: http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
This looks good. The one thing I find concerning is running terminate in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. ^self suspend].
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend for termination'].
-- Eliot's fix of infinite recursion in doesNotUnderstand:
doesNotUnderstand: aMessage "Handle the fact that there was an attempt to send the given message to the receiver but the receiver does not understand this message (typically sent from the machine when a message is sent to the receiver and no method is defined for that selector)."
"Testing: (3 activeProcess)" | exception resumeValue | (exception := MessageNotUnderstood new) message: aMessage; receiver: self. resumeValue := exception signal. ^exception reachedDefaultHandler ifFalse: [resumeValue]
"----------> this wrapper detecting recursion is added: -------------->"
ifTrue: [ [aMessage sentTo: self] on: MessageNotUnderstood do: [:ex| | args | args := ex message arguments. (ex receiver == self and: [ex message selector ==
aMessage selector and: [(1 to: aMessage numArgs) allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) ifFalse: [ex pass]. self error: 'infinite recursion in doesNotUnderstand:']]
(see discussion http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.... )
best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote:
Hi Christoph,
Once more, apologies for taking so long to respond.
[...] there really are two distinct unwind semantics : one "light"
for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to pack the
two twin beasts into one can (I refer to "rueorf" & "ruueorf"). Also, we could consider Process >> complete:to: as a generalized version of Process
complete and rewrite it accordingly. This is doable, maybe even
desirable but for the time being I'd very much prefer to keep them separate to facilitate troubleshooting of potential problems (or localizing them). Each of the two similar methods belong to a different execution path after all. And besides, #runUntilErrorOrReturnFrom is already crazy enough and I don't feel like making it even more complex now :)
With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw and
deserves its own thread.
Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
[...]- could you maybe point me to some concrete examples that lead to
an infinite recursion without this special check? :-)
Try:
[self error: 'error'] ensure: [self gotcha. Transcript show: 'all
cleaned up! ']
'gotcha' represents a method unknown to the system. The code will print
'all cleaned up! ' after abandoning the error and the debugger - meaning the system will survive this code and execute all cleanups (aka unwinds). In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course because you skip completing the halfway through unwind. What I'm saying is: even if you chose the classic heavy #terminate to exit the Debugger, the code will execute remaining unwinds as one might expect (or hope for).
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you
maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected soon
anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the
BlockCannotReturn behavior here:
Isn't that ProceedBlockCannotReturn tautologous? I think that by
actively proceeding from a BlockCannotReturn error users already accept that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of
violates common sense but during our lengthy discussion you convinced me it makes a very good sense when troubleshooting :) The idea is by no means trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the process
continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >> #complete:to: to deal with the new course of events.
Apart from that, the message text of your new warning is not correct
if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even if
someone hit Proceed without much thinking they could get an idea what's about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox: Kernel-ct.1405.mcz"
thread and we can continue discussing this particular issue there. It's really a separate issue and I included your patch here along with the main termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
[...] the computation terminated is also wrong, IMO, you should get a
BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be
discussed thoroughly and separately in the "The Inbox: Kernel-ct.1405.mcz" thread.
If you agree I'd really appreciate if your fix could be accepted here in
including the ProceedBlockCannotReturn exception I need to make it work together with the code I'm presenting here. If we come up with a better idea in the other discussion we can make amends here as well; I take it as a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's always
a pleasure :) Plus - the Inbox Talk is priceless... I use it exclusively now and look forward to any improvements you might come up in the future ;)
Best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede at
student.hpi.uni-potsdam.de wrote:
Hi Jaromir,
Yes, I was wondering why I couldn't get rid of the duplication and
now I think it's because there really are two distinct unwind semantics : one "light" for regular returns and one "heavy" for termination. Both are very similar yet each require a slightly different behavior - that's why the duality #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated code very often tends to be harder to maintained.
With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with non-local returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
But in general - yes, any method/exception purposefully (or not)
written to create a loop will break this patch (I admit it is just a patch really). I extracted it to #complete:to: to make #terminate clean; this is a WIP; I wish there was a holistic solution to this - maybe checking for exception recursion by default? :)
Sounds better already, if feasible! But how would you detect this?
Unfortunately I've lost track of these infinite loops - could you maybe point me to some concrete examples that lead to an infinite recursion without this special check? :-)
One more question about your #runUnwindUntilErrorOrReturnFrom: Are you
maybe missing something like "cxt terminate" in the "No error was raised" case? Just wondering.
Best, Christoph
[1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
Sent from Squeak Inbox Talk
On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote:
Jaromir Matas wrote > Hi All, > I've sent an updated version of #teminate integrating Christoph's
solution
> of BlockCannotReturn recursion problem (in [1]), along with a
battery of
> tests exploring termination of nested ensure and cascading errors
behavior
> (Debugger tests are for info and a final version can wait until
releasing
> Christoph's proposal in [2]). > > It's pretty much final, I hope... > > Any complaints about #terminate - please shout ;) > > [1]
http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html
> [2] >
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
> > best,
Here's the link: http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
^[^ Jaromir
Sent from: http://forum.world.st/Squeak-Dev-f45488.html
On Mon, Nov 29, 2021 at 1:46 AM mail@jaromir.net wrote:
Hi Eliot, all,
This looks good. The one thing I find concerning is running terminate in
a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess
ifAbsent: []]
Would this test prove #terminate works as you'd expect?
| q | [ [q := Processor activeProcess. Processor activeProcess suspend] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield. q terminate
Yes, that captures it.
Or with a raised exception:
| q | [ [q := Processor activeProcess. self error] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield.
(If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, indeed)
Please let me know if this is acceptable.
ish. The reason I thought you weren't setting the effective process is that it's buried in complete:to:. I'd rather see it up front in Process>>terminate itself. But I'm happy to defer to you; you've done the work :-)
Thanks again,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T22:57:25+01:00, mail@jaromir.net wrote:
Hi Eliot,
thanks very much for reviewing this.
This looks good. The one thing I find concerning is running terminate
in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created.
Frankly, I haven't explicitly considered that... In the worst case we
could revert active process termination bit like it was. Terminating an active process via a different process was just supposed to unify (and extend) the termination semantics.
At the very least shouldn't the effectiveProcess be set to be that of
the process that is terminating while its unwind block executes?
That's what I though I did in Process>>#complete: topContext to:
aContext
[...] pair := Processor activeProcess evaluate: [topContext
runUnwindUntilErrorOrReturnFrom: aContext]
onBehalfOf: self.
[...]
where self is the process being terminated.
#runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being terminated and executes the unwind block with effective process set to the process being terminated.
Or am I mistaken?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor
activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created
seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
This is a very interesting example - thanks! Running terminate in a
different process allows to relatively easily identify all unwind blocks half-way through their execution and complete them. In case setting the effective process in #complete:to: doesn't work as you requested, then I'm afraid unwinding blocks half-way through their execution would be very hard to achieve from the process being terminated itself (not impossible though
- I tried but then doubted it was worth the effort). Or theoretically, it
might be possible to maintain two stacks for the process being terminated - one "original" that needs to be unwound and a second "helper" one running terminate - but I haven't gotten beyond the idea, it seemed too unchartered :)
Besides, when an error happens during unwind, a debugger opens and the
identity of the process running terminate may change, so the invariant may not survive the first error anyway (at least in what I was considering).
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess.
arnold resume. ^self suspend].
No, that wouldn't work because the Terminator identifies itself as the
process being terminated when executing [self terminate] resulting in an infinite loop.
At the moment I can't see why setting the effective process in
#complete:to: wouldn't suffice but I'll get there :) I'll try to come up with a test.
I'm intrigued: why did you use '[arnold evaluate:...' and not
'[Processor activeProcess evaluate:...'; I can't figure out the difference :)
I might also consider discarding the return of suspend, because it
shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess.
arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend
for termination'].
No, this wouldn't work if Processor activeProcess terminate was inside
an unwind block. Terminate would attempt to proceed after self suspend and would always raise the error.
Thanks again,
Jaromir
PS: I used this example to test your suggestions:
| p | p := [ [ [ ] ensure: [ [Processor activeProcess terminate] ensure: [ Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3']. Transcript show: 'x4' ] newProcess. p resume. "Two yields necessary: terminate active is a two-step procedure" Processor yield. Processor yield. Transcript show: p isTerminated printString "prints x1 x2 x3"
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote:
Hi Jaromir, Hi Christoph,
On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote:
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to
Eliot's
solution of the infinite recursion in #doesNotUnderstand the
#terminate
code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead
(because I'm
not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final
version of
my effort to get rid of all the bugs reported in this thread (and
make
#terminate semantics more consistent). It passes all standard tests
and the
tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the
'block
cannot return issue' presented in Kernel-ct.1405 (
http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html ).
Summary and discussion about the bugs and changes in #terminate:
http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
This looks good. The one thing I find concerning is running terminate
in a
different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor
activeProcess
ifAbsent: []]
That unwind blocks are run in the same process as they were created
seems
to me to be a legitimate expectation. Can this be reengineered
maintaining
this invariant, or at least setting the effective process to be that
of the
process being terminated?
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. ^self suspend].
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after
suspend
for termination'].
-- Eliot's fix of infinite recursion in doesNotUnderstand:
doesNotUnderstand: aMessage "Handle the fact that there was an attempt to send the given message to the receiver but the receiver does not
understand
this message (typically sent from the machine when a
message
is sent to the receiver and no method is defined for that
selector)."
"Testing: (3 activeProcess)" | exception resumeValue | (exception := MessageNotUnderstood new) message: aMessage; receiver: self. resumeValue := exception signal. ^exception reachedDefaultHandler ifFalse: [resumeValue]
"----------> this wrapper detecting recursion is added:
-------------->"
ifTrue: [ [aMessage sentTo: self] on: MessageNotUnderstood do: [:ex| | args | args := ex message arguments. (ex receiver == self and: [ex message selector ==
aMessage selector and: [(1 to: aMessage
numArgs)
allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]])
ifFalse:
[ex pass]. self error: 'infinite recursion
in
doesNotUnderstand:']]
(see discussion
http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031....
)
best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote:
Hi Christoph,
Once more, apologies for taking so long to respond.
> [...] there really are two distinct unwind semantics : one
"light"
for regular returns and one "heavy" for termination. Both are very
similar
yet each require a slightly different behavior - that's why the
duality
#runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or
#complete:
/ #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some
extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated
code
very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to
pack the
two twin beasts into one can (I refer to "rueorf" & "ruueorf").
Also, we
could consider Process >> complete:to: as a generalized version of
Process
complete and rewrite it accordingly. This is doable, maybe even
desirable but for the time being I'd very much prefer to keep them
separate
to facilitate troubleshooting of potential problems (or localizing
them).
Each of the two similar methods belong to a different execution path
after
all. And besides, #runUntilErrorOrReturnFrom is already crazy enough
and I
don't feel like making it even more complex now :)
> With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with
non-local
returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as
described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw
and
deserves its own thread.
Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
[...]- could you maybe point me to some concrete examples that
lead to
an infinite recursion without this special check? :-)
Try:
[self error: 'error'] ensure: [self gotcha. Transcript show: 'all
cleaned up! ']
'gotcha' represents a method unknown to the system. The code will
'all cleaned up! ' after abandoning the error and the debugger -
meaning
the system will survive this code and execute all cleanups (aka
unwinds).
In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course
because you
skip completing the halfway through unwind. What I'm saying is: even
if you
chose the classic heavy #terminate to exit the Debugger, the code
will
execute remaining unwinds as one might expect (or hope for).
One more question about your #runUnwindUntilErrorOrReturnFrom:
Are you
maybe missing something like "cxt terminate" in the "No error was
raised"
case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected
soon
anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the
BlockCannotReturn behavior here:
Isn't that ProceedBlockCannotReturn tautologous? I think that by
actively proceeding from a BlockCannotReturn error users already
accept
that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of
violates common sense but during our lengthy discussion you
convinced me it
makes a very good sense when troubleshooting :) The idea is by no
means
trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the
process
continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >>
#complete:to: to
deal with the new course of events.
Apart from that, the message text of your new warning is not
correct
if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even
if
someone hit Proceed without much thinking they could get an idea
what's
about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox:
Kernel-ct.1405.mcz"
thread and we can continue discussing this particular issue there.
It's
really a separate issue and I included your patch here along with
the main
termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
[...] the computation terminated is also wrong, IMO, you should
get a
BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be
discussed thoroughly and separately in the "The Inbox:
Kernel-ct.1405.mcz"
thread.
If you agree I'd really appreciate if your fix could be accepted
here in
including the ProceedBlockCannotReturn exception I need to make it
work
together with the code I'm presenting here. If we come up with a
better
idea in the other discussion we can make amends here as well; I take
it as
a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's
always
a pleasure :) Plus - the Inbox Talk is priceless... I use it
exclusively
now and look forward to any improvements you might come up in the
future ;)
Best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede at
student.hpi.uni-potsdam.de wrote:
Hi Jaromir,
> Yes, I was wondering why I couldn't get rid of the duplication
and
now I think it's because there really are two distinct unwind
semantics :
one "light" for regular returns and one "heavy" for termination.
Both are
very similar yet each require a slightly different behavior - that's
why
the duality #runUntilErrorOrReturnFrom /
#runUnwindUntilErrorOrReturnFrom
or #complete: / #complete:to: and #unwindTo: / #terminate.
But they are still pretty similar ... Couldn't you just add some
extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated
code
very often tends to be harder to maintained.
> With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with
non-local
returns as the original #terminate and require a similar fix?
Hm, do we need to implement both modi - (ii) and (iii) as
described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
> But in general - yes, any method/exception purposefully (or
not)
written to create a loop will break this patch (I admit it is just a
patch
really). I extracted it to #complete:to: to make #terminate clean;
this is
a WIP; I wish there was a holistic solution to this - maybe checking
for
exception recursion by default? :)
Sounds better already, if feasible! But how would you detect
this?
Unfortunately I've lost track of these infinite loops - could you
maybe
point me to some concrete examples that lead to an infinite recursion without this special check? :-)
One more question about your #runUnwindUntilErrorOrReturnFrom:
Are you
maybe missing something like "cxt terminate" in the "No error was
raised"
case? Just wondering.
Best, Christoph
[1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
Sent from Squeak Inbox Talk
On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote:
> Jaromir Matas wrote > > Hi All, > > I've sent an updated version of #teminate integrating
Christoph's
solution
> > of BlockCannotReturn recursion problem (in [1]), along with a
battery of
> > tests exploring termination of nested ensure and cascading
errors
behavior
> > (Debugger tests are for info and a final version can wait
until
releasing
> > Christoph's proposal in [2]). > > > > It's pretty much final, I hope... > > > > Any complaints about #terminate - please shout ;) > > > > [1]
http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html
> > [2] > >
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
> > > > best, > > Here's the link: >
http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
> > > > ----- > ^[^ Jaromir > -- > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > >
On 2021-12-02T17:37:17-08:00, eliot.miranda@gmail.com wrote:
On Mon, Nov 29, 2021 at 1:46 AM <mail at jaromir.net> wrote:
Hi Eliot, all,
This looks good. The one thing I find concerning is running terminate in
a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess
ifAbsent: []]
Would this test prove #terminate works as you'd expect?
| q | [ [q := Processor activeProcess. Processor activeProcess suspend] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield. q terminate
Yes, that captures it.
Or with a raised exception:
| q | [ [q := Processor activeProcess. self error] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield.
(If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, indeed)
Please let me know if this is acceptable.
ish. The reason I thought you weren't setting the effective process is that it's buried in complete:to:. I'd rather see it up front in Process>>terminate itself. But I'm happy to defer to you; you've done the work :-)
Thanks very much for your feedback; it's really important for me!
I placed setting the effective process into #complete:to: for two reasons: (1) #complete:to: is called twice in #terminate (-> remove duplicity) (2) #complete:to: has been inspired by #complete and tried to retain the similarity :)
Best regards, Jaromir
Thanks again,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T22:57:25+01:00, mail at jaromir.net wrote:
Hi Eliot,
thanks very much for reviewing this.
This looks good. The one thing I find concerning is running terminate
in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created.
Frankly, I haven't explicitly considered that... In the worst case we
could revert active process termination bit like it was. Terminating an active process via a different process was just supposed to unify (and extend) the termination semantics.
At the very least shouldn't the effectiveProcess be set to be that of
the process that is terminating while its unwind block executes?
That's what I though I did in Process>>#complete: topContext to:
aContext
[...] pair := Processor activeProcess evaluate: [topContext
runUnwindUntilErrorOrReturnFrom: aContext]
onBehalfOf: self.
[...]
where self is the process being terminated.
#runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being terminated and executes the unwind block with effective process set to the process being terminated.
Or am I mistaken?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor
activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created
seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
This is a very interesting example - thanks! Running terminate in a
different process allows to relatively easily identify all unwind blocks half-way through their execution and complete them. In case setting the effective process in #complete:to: doesn't work as you requested, then I'm afraid unwinding blocks half-way through their execution would be very hard to achieve from the process being terminated itself (not impossible though
- I tried but then doubted it was worth the effort). Or theoretically, it
might be possible to maintain two stacks for the process being terminated - one "original" that needs to be unwound and a second "helper" one running terminate - but I haven't gotten beyond the idea, it seemed too unchartered :)
Besides, when an error happens during unwind, a debugger opens and the
identity of the process running terminate may change, so the invariant may not survive the first error anyway (at least in what I was considering).
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess.
arnold resume. ^self suspend].
No, that wouldn't work because the Terminator identifies itself as the
process being terminated when executing [self terminate] resulting in an infinite loop.
At the moment I can't see why setting the effective process in
#complete:to: wouldn't suffice but I'll get there :) I'll try to come up with a test.
I'm intrigued: why did you use '[arnold evaluate:...' and not
'[Processor activeProcess evaluate:...'; I can't figure out the difference :)
I might also consider discarding the return of suspend, because it
shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess.
arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend
for termination'].
No, this wouldn't work if Processor activeProcess terminate was inside
an unwind block. Terminate would attempt to proceed after self suspend and would always raise the error.
Thanks again,
Jaromir
PS: I used this example to test your suggestions:
| p | p := [ [ [ ] ensure: [ [Processor activeProcess terminate] ensure: [ Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3']. Transcript show: 'x4' ] newProcess. p resume. "Two yields necessary: terminate active is a two-step procedure" Processor yield. Processor yield. Transcript show: p isTerminated printString "prints x1 x2 x3"
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote:
Hi Jaromir, Hi Christoph,
On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote:
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to
Eliot's
solution of the infinite recursion in #doesNotUnderstand the
#terminate
code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead
(because I'm
not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final
version of
my effort to get rid of all the bugs reported in this thread (and
make
#terminate semantics more consistent). It passes all standard tests
and the
tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the
'block
cannot return issue' presented in Kernel-ct.1405 (
http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html ).
Summary and discussion about the bugs and changes in #terminate:
http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
This looks good. The one thing I find concerning is running terminate
in a
different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor
activeProcess
ifAbsent: []]
That unwind blocks are run in the same process as they were created
seems
to me to be a legitimate expectation. Can this be reengineered
maintaining
this invariant, or at least setting the effective process to be that
of the
process being terminated?
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. ^self suspend].
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after
suspend
for termination'].
-- Eliot's fix of infinite recursion in doesNotUnderstand:
doesNotUnderstand: aMessage "Handle the fact that there was an attempt to send the given message to the receiver but the receiver does not
understand
this message (typically sent from the machine when a
message
is sent to the receiver and no method is defined for that
selector)."
"Testing: (3 activeProcess)" | exception resumeValue | (exception := MessageNotUnderstood new) message: aMessage; receiver: self. resumeValue := exception signal. ^exception reachedDefaultHandler ifFalse: [resumeValue]
"----------> this wrapper detecting recursion is added:
-------------->"
ifTrue: [ [aMessage sentTo: self] on: MessageNotUnderstood do: [:ex| | args | args := ex message arguments. (ex receiver == self and: [ex message selector ==
aMessage selector and: [(1 to: aMessage
numArgs)
allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]])
ifFalse:
[ex pass]. self error: 'infinite recursion
in
doesNotUnderstand:']]
(see discussion
http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031....
)
best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote:
Hi Christoph,
Once more, apologies for taking so long to respond.
> > [...] there really are two distinct unwind semantics : one
"light"
for regular returns and one "heavy" for termination. Both are very
similar
yet each require a slightly different behavior - that's why the
duality
#runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or
#complete:
/ #complete:to: and #unwindTo: / #terminate.
> > But they are still pretty similar ... Couldn't you just add some
extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated
code
very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to
pack the
two twin beasts into one can (I refer to "rueorf" & "ruueorf").
Also, we
could consider Process >> complete:to: as a generalized version of
Process
> complete and rewrite it accordingly. This is doable, maybe even
desirable but for the time being I'd very much prefer to keep them
separate
to facilitate troubleshooting of potential problems (or localizing
them).
Each of the two similar methods belong to a different execution path
after
all. And besides, #runUntilErrorOrReturnFrom is already crazy enough
and I
don't feel like making it even more complex now :)
> > With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with
non-local
returns as the original #terminate and require a similar fix?
> > Hm, do we need to implement both modi - (ii) and (iii) as
described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw
and
deserves its own thread.
> Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
> [...]- could you maybe point me to some concrete examples that
lead to
an infinite recursion without this special check? :-)
Try:
[self error: 'error'] ensure: [self gotcha. Transcript show: 'all
cleaned up! ']
'gotcha' represents a method unknown to the system. The code will
'all cleaned up! ' after abandoning the error and the debugger -
meaning
the system will survive this code and execute all cleanups (aka
unwinds).
In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course
because you
skip completing the halfway through unwind. What I'm saying is: even
if you
chose the classic heavy #terminate to exit the Debugger, the code
will
execute remaining unwinds as one might expect (or hope for).
> One more question about your #runUnwindUntilErrorOrReturnFrom:
Are you
maybe missing something like "cxt terminate" in the "No error was
raised"
case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected
soon
anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the
BlockCannotReturn behavior here:
> Isn't that ProceedBlockCannotReturn tautologous? I think that by
actively proceeding from a BlockCannotReturn error users already
accept
that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of
violates common sense but during our lengthy discussion you
convinced me it
makes a very good sense when troubleshooting :) The idea is by no
means
trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the
process
continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >>
#complete:to: to
deal with the new course of events.
> Apart from that, the message text of your new warning is not
correct
if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even
if
someone hit Proceed without much thinking they could get an idea
what's
about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox:
Kernel-ct.1405.mcz"
thread and we can continue discussing this particular issue there.
It's
really a separate issue and I included your patch here along with
the main
termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
> [...] the computation terminated is also wrong, IMO, you should
get a
BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be
discussed thoroughly and separately in the "The Inbox:
Kernel-ct.1405.mcz"
thread.
If you agree I'd really appreciate if your fix could be accepted
here in
including the ProceedBlockCannotReturn exception I need to make it
work
together with the code I'm presenting here. If we come up with a
better
idea in the other discussion we can make amends here as well; I take
it as
a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's
always
a pleasure :) Plus - the Inbox Talk is priceless... I use it
exclusively
now and look forward to any improvements you might come up in the
future ;)
Best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede at
student.hpi.uni-potsdam.de wrote:
> Hi Jaromir, > > > Yes, I was wondering why I couldn't get rid of the duplication
and
now I think it's because there really are two distinct unwind
semantics :
one "light" for regular returns and one "heavy" for termination.
Both are
very similar yet each require a slightly different behavior - that's
why
the duality #runUntilErrorOrReturnFrom /
#runUnwindUntilErrorOrReturnFrom
or #complete: / #complete:to: and #unwindTo: / #terminate.
> > But they are still pretty similar ... Couldn't you just add some
extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated
code
very often tends to be harder to maintained.
> > > With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with
non-local
returns as the original #terminate and require a similar fix?
> > Hm, do we need to implement both modi - (ii) and (iii) as
described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
> > > But in general - yes, any method/exception purposefully (or
not)
written to create a loop will break this patch (I admit it is just a
patch
really). I extracted it to #complete:to: to make #terminate clean;
this is
a WIP; I wish there was a holistic solution to this - maybe checking
for
exception recursion by default? :)
> > Sounds better already, if feasible! But how would you detect
this?
Unfortunately I've lost track of these infinite loops - could you
maybe
point me to some concrete examples that lead to an infinite recursion without this special check? :-)
> > One more question about your #runUnwindUntilErrorOrReturnFrom:
Are you
maybe missing something like "cxt terminate" in the "No error was
raised"
case? Just wondering.
> > Best, > Christoph > > [1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
> > --- > Sent from Squeak Inbox Talk > > On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote: > > > Jaromir Matas wrote > > > Hi All, > > > I've sent an updated version of #teminate integrating
Christoph's
solution
> > > of BlockCannotReturn recursion problem (in [1]), along with a
battery of
> > > tests exploring termination of nested ensure and cascading
errors
behavior
> > > (Debugger tests are for info and a final version can wait
until
releasing
> > > Christoph's proposal in [2]). > > > > > > It's pretty much final, I hope... > > > > > > Any complaints about #terminate - please shout ;) > > > > > > [1]
http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html
> > > [2] > > >
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
> > > > > > best, > > > > Here's the link: > >
http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
> > > > > > > > ----- > > ^[^ Jaromir > > -- > > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > > > > >
Hi Eliot, Christoph, all
Thanks to Eliot's remarks I substantially simplified #terminate's helper method #runUnwind:onBehalfOf: (former #complete:to:) and updated comments. The functionality remains unchanged and all tests pass.
Tested also with the new VM 202112022203.
See Kernel-jar.1434
Thanks for your comments, best,
~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-12-02T17:37:17-08:00, eliot.miranda@gmail.com wrote:
On Mon, Nov 29, 2021 at 1:46 AM <mail at jaromir.net> wrote:
Hi Eliot, all,
This looks good. The one thing I find concerning is running terminate in
a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess
ifAbsent: []]
Would this test prove #terminate works as you'd expect?
| q | [ [q := Processor activeProcess. Processor activeProcess suspend] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield. q terminate
Yes, that captures it.
Or with a raised exception:
| q | [ [q := Processor activeProcess. self error] ensure: [ self assert: q = Processor activeProcess] ] fork. Processor yield.
(If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, indeed)
Please let me know if this is acceptable.
ish. The reason I thought you weren't setting the effective process is that it's buried in complete:to:. I'd rather see it up front in Process>>terminate itself. But I'm happy to defer to you; you've done the work :-)
Thanks again,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T22:57:25+01:00, mail at jaromir.net wrote:
Hi Eliot,
thanks very much for reviewing this.
This looks good. The one thing I find concerning is running terminate
in a different process. Various idioms could legitimately require an unwind block to run in the process in which it was created.
Frankly, I haven't explicitly considered that... In the worst case we
could revert active process termination bit like it was. Terminating an active process via a different process was just supposed to unify (and extend) the termination semantics.
At the very least shouldn't the effectiveProcess be set to be that of
the process that is terminating while its unwind block executes?
That's what I though I did in Process>>#complete: topContext to:
aContext
[...] pair := Processor activeProcess evaluate: [topContext
runUnwindUntilErrorOrReturnFrom: aContext]
onBehalfOf: self.
[...]
where self is the process being terminated.
#runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being terminated and executes the unwind block with effective process set to the process being terminated.
Or am I mistaken?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor
activeProcess ifAbsent: []]
That unwind blocks are run in the same process as they were created
seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant, or at least setting the effective process to be that of the process being terminated?
This is a very interesting example - thanks! Running terminate in a
different process allows to relatively easily identify all unwind blocks half-way through their execution and complete them. In case setting the effective process in #complete:to: doesn't work as you requested, then I'm afraid unwinding blocks half-way through their execution would be very hard to achieve from the process being terminated itself (not impossible though
- I tried but then doubted it was worth the effort). Or theoretically, it
might be possible to maintain two stacks for the process being terminated - one "original" that needs to be unwound and a second "helper" one running terminate - but I haven't gotten beyond the idea, it seemed too unchartered :)
Besides, when an error happens during unwind, a debugger opens and the
identity of the process running terminate may change, so the invariant may not survive the first error anyway (at least in what I was considering).
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess.
arnold resume. ^self suspend].
No, that wouldn't work because the Terminator identifies itself as the
process being terminated when executing [self terminate] resulting in an infinite loop.
At the moment I can't see why setting the effective process in
#complete:to: wouldn't suffice but I'll get there :) I'll try to come up with a test.
I'm intrigued: why did you use '[arnold evaluate:...' and not
'[Processor activeProcess evaluate:...'; I can't figure out the difference :)
I might also consider discarding the return of suspend, because it
shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess.
arnold resume. self suspend. self error: 'error in terminate; execution resumed after suspend
for termination'].
No, this wouldn't work if Processor activeProcess terminate was inside
an unwind block. Terminate would attempt to proceed after self suspend and would always raise the error.
Thanks again,
Jaromir
PS: I used this example to test your suggestions:
| p | p := [ [ [ ] ensure: [ [Processor activeProcess terminate] ensure: [ Transcript show: 'x1']. Transcript show: 'x2'] ] ensure: [ Transcript show: 'x3']. Transcript show: 'x4' ] newProcess. p resume. "Two yields necessary: terminate active is a two-step procedure" Processor yield. Processor yield. Transcript show: p isTerminated printString "prints x1 x2 x3"
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote:
Hi Jaromir, Hi Christoph,
On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote:
Hi all, Christoph,
I finally managed to separate #terminate and the 'block cannot return issue' so that we can discuss them independently :) And thanks to
Eliot's
solution of the infinite recursion in #doesNotUnderstand the
#terminate
code is now cleaner. I haven't included Eliot's modification of #doesNotUnderstand in my changeset but paste it below instead
(because I'm
not the author; I hope it'll be merged though):
The #terminate code presented in Kernel-jar.1426 is the final
version of
my effort to get rid of all the bugs reported in this thread (and
make
#terminate semantics more consistent). It passes all standard tests
and the
tests complementing this changeset:
KernelTests-jar.406 (Terminator test) KernelTests-jar.407 (McClure test) Tests-jar.466 (unwind tests) ToolsTests-jar.105 (debugger tests)
In addition, it's fully compatible with Christoph's solution of the
'block
cannot return issue' presented in Kernel-ct.1405 (
http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html ).
Summary and discussion about the bugs and changes in #terminate:
http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal...
I think it's ready for merging... Please review.
This looks good. The one thing I find concerning is running terminate
in a
different process. Various idioms could legitimately require an unwind block to run in the process in which it was created. At the very least shouldn't the effectiveProcess be set to be that of the process that is terminating while its unwind block executes?
For example, what if I did something like this: doWhileMonitoring: aBlock monitoredProcesses add: Processor activeProcess. ^aBlock ensure: [monitoredProcesses remove:: Processor
activeProcess
ifAbsent: []]
That unwind blocks are run in the same process as they were created
seems
to me to be a legitimate expectation. Can this be reengineered
maintaining
this invariant, or at least setting the effective process to be that
of the
process being terminated?
So instead of
self isActiveProcess ifTrue: [ [self terminate] fork. ^self suspend].
we would use
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. ^self suspend].
I might also consider discarding the return of suspend, because it shouldn't be executed normally, and using, say
self isActiveProcess ifTrue: [| arnold | arnold := [arnold evaluate: [self terminate] onBehalfOf: self]
newProcess. arnold resume. self suspend. self error: 'error in terminate; execution resumed after
suspend
for termination'].
-- Eliot's fix of infinite recursion in doesNotUnderstand:
doesNotUnderstand: aMessage "Handle the fact that there was an attempt to send the given message to the receiver but the receiver does not
understand
this message (typically sent from the machine when a
message
is sent to the receiver and no method is defined for that
selector)."
"Testing: (3 activeProcess)" | exception resumeValue | (exception := MessageNotUnderstood new) message: aMessage; receiver: self. resumeValue := exception signal. ^exception reachedDefaultHandler ifFalse: [resumeValue]
"----------> this wrapper detecting recursion is added:
-------------->"
ifTrue: [ [aMessage sentTo: self] on: MessageNotUnderstood do: [:ex| | args | args := ex message arguments. (ex receiver == self and: [ex message selector ==
aMessage selector and: [(1 to: aMessage
numArgs)
allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]])
ifFalse:
[ex pass]. self error: 'infinite recursion
in
doesNotUnderstand:']]
(see discussion
http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031....
)
best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote:
Hi Christoph,
Once more, apologies for taking so long to respond.
> > [...] there really are two distinct unwind semantics : one
"light"
for regular returns and one "heavy" for termination. Both are very
similar
yet each require a slightly different behavior - that's why the
duality
#runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or
#complete:
/ #complete:to: and #unwindTo: / #terminate.
> > But they are still pretty similar ... Couldn't you just add some
extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated
code
very often tends to be harder to maintained.
Yes, I understand your concern and agree two ifs would allow to
pack the
two twin beasts into one can (I refer to "rueorf" & "ruueorf").
Also, we
could consider Process >> complete:to: as a generalized version of
Process
> complete and rewrite it accordingly. This is doable, maybe even
desirable but for the time being I'd very much prefer to keep them
separate
to facilitate troubleshooting of potential problems (or localizing
them).
Each of the two similar methods belong to a different execution path
after
all. And besides, #runUntilErrorOrReturnFrom is already crazy enough
and I
don't feel like making it even more complex now :)
> > With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with
non-local
returns as the original #terminate and require a similar fix?
> > Hm, do we need to implement both modi - (ii) and (iii) as
described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
I put #unwindTo: aside for the moment; it contains the same flaw
and
deserves its own thread.
> Unfortunately I've lost track of these infinite loops [...]
Yeah, tell me about it :)
> [...]- could you maybe point me to some concrete examples that
lead to
an infinite recursion without this special check? :-)
Try:
[self error: 'error'] ensure: [self gotcha. Transcript show: 'all
cleaned up! ']
'gotcha' represents a method unknown to the system. The code will
'all cleaned up! ' after abandoning the error and the debugger -
meaning
the system will survive this code and execute all cleanups (aka
unwinds).
In case you use your #terminateAggressively as the default action for abandoning the Debugger, you won't have any problem of course
because you
skip completing the halfway through unwind. What I'm saying is: even
if you
chose the classic heavy #terminate to exit the Debugger, the code
will
execute remaining unwinds as one might expect (or hope for).
> One more question about your #runUnwindUntilErrorOrReturnFrom:
Are you
maybe missing something like "cxt terminate" in the "No error was
raised"
case? Just wondering.
I guess I know what you mean... 'ctxt' would get garbage collected
soon
anyway so I left it as is to keep the code as simple as possible.
Christoph, I'll also address your comments regarding the
BlockCannotReturn behavior here:
> Isn't that ProceedBlockCannotReturn tautologous? I think that by
actively proceeding from a BlockCannotReturn error users already
accept
that they are going to resume execution in another way.
Well, the very proceeding from a BlockCannotReturn error sort of
violates common sense but during our lengthy discussion you
convinced me it
makes a very good sense when troubleshooting :) The idea is by no
means
trivial - unlike hitting Proceed :) So an extra warning can't hurt...
But more importantly, I need something to know the user let the
process
continue after reaching the BlockCannotReturn error - thus the new ProceedBlockCannotReturn exception which allows Process >>
#complete:to: to
deal with the new course of events.
> Apart from that, the message text of your new warning is not
correct
if self pc <= self endPC. :-)
Yes, and I'd like to make the warning message more verbose so even
if
someone hit Proceed without much thinking they could get an idea
what's
about to happen :) Should the warning interfere with some potential automation efforts we could come up with some alternative way.
I'll copy my reply regarding BCR to the "The Inbox:
Kernel-ct.1405.mcz"
thread and we can continue discussing this particular issue there.
It's
really a separate issue and I included your patch here along with
the main
termination code because it perfectly complements it and prevents the disastrous crashes caused by Proceeding the BCR error.
> [...] the computation terminated is also wrong, IMO, you should
get a
BlockCannotReturn here.
Yes, I agree, there's definitely more to it which deserves to be
discussed thoroughly and separately in the "The Inbox:
Kernel-ct.1405.mcz"
thread.
If you agree I'd really appreciate if your fix could be accepted
here in
including the ProceedBlockCannotReturn exception I need to make it
work
together with the code I'm presenting here. If we come up with a
better
idea in the other discussion we can make amends here as well; I take
it as
a patch, not a definitive solution :)
Thanks again so much for all your suggestions and examples! It's
always
a pleasure :) Plus - the Inbox Talk is priceless... I use it
exclusively
now and look forward to any improvements you might come up in the
future ;)
Best,
^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-08-22T16:49:10+02:00, christoph.thiede at
student.hpi.uni-potsdam.de wrote:
> Hi Jaromir, > > > Yes, I was wondering why I couldn't get rid of the duplication
and
now I think it's because there really are two distinct unwind
semantics :
one "light" for regular returns and one "heavy" for termination.
Both are
very similar yet each require a slightly different behavior - that's
why
the duality #runUntilErrorOrReturnFrom /
#runUnwindUntilErrorOrReturnFrom
or #complete: / #complete:to: and #unwindTo: / #terminate.
> > But they are still pretty similar ... Couldn't you just add some
extra
parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: aggressive"? I have the feeling that we could eliminate significant duplication that way by inserting some ifs and elses ... Duplicated
code
very often tends to be harder to maintained.
> > > With regards to #unwindTo: - I haven't tested it yet but I'm
wondering whether it wouldn't have the same unwind problem with
non-local
returns as the original #terminate and require a similar fix?
> > Hm, do we need to implement both modi - (ii) and (iii) as
described in
[1] - in Context >> #unwindTo: as well? Something like #unwindTo:aggressive:?
> > > But in general - yes, any method/exception purposefully (or
not)
written to create a loop will break this patch (I admit it is just a
patch
really). I extracted it to #complete:to: to make #terminate clean;
this is
a WIP; I wish there was a holistic solution to this - maybe checking
for
exception recursion by default? :)
> > Sounds better already, if feasible! But how would you detect
this?
Unfortunately I've lost track of these infinite loops - could you
maybe
point me to some concrete examples that lead to an infinite recursion without this special check? :-)
> > One more question about your #runUnwindUntilErrorOrReturnFrom:
Are you
maybe missing something like "cxt terminate" in the "No error was
raised"
case? Just wondering.
> > Best, > Christoph > > [1]
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
> > --- > Sent from Squeak Inbox Talk > > On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote: > > > Jaromir Matas wrote > > > Hi All, > > > I've sent an updated version of #teminate integrating
Christoph's
solution
> > > of BlockCannotReturn recursion problem (in [1]), along with a
battery of
> > > tests exploring termination of nested ensure and cascading
errors
behavior
> > > (Debugger tests are for info and a final version can wait
until
releasing
> > > Christoph's proposal in [2]). > > > > > > It's pretty much final, I hope... > > > > > > Any complaints about #terminate - please shout ;) > > > > > > [1]
http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html
> > > [2] > > >
http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-duri...
> > > > > > best, > > > > Here's the link: > >
http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html
> > > > > > > > ----- > > ^[^ Jaromir > > -- > > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > > > > >
Hi All, Christoph, Eliot,
One more update of new #terminate; I'm sorry for further extending this thread. I've rediscovered how very tricky debugging the BCR is; take this example with nested ensure blocks containg non-local returns:
[[[] ensure: [^2]] ensure: [^42]] fork
#terminate needs to avoid returning from the two dangerous contexts while still unwinding as much as possible without a crash :) Also, there's one ensure block inside the debugger opening sequence (inside Debugger class>>openOn:context:... ) which needs to be executed when abandoning the debugger. (I always forget the debugger opening sequence on the top of the stack of the process being debugged is hidden by the debugger)
I hope the latest #terminate in Kernel-jar.1435 deals with all these issues correctly at last (on top off all other issues and bugs).
Many thanks for your comments or a review.
best,
~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-12-10T13:45:26+01:00, mail@jaromir.net wrote:
Hi Eliot, Christoph, all
Thanks to Eliot's remarks I substantially simplified #terminate's helper method #runUnwind:onBehalfOf: (former #complete:to:) and updated comments. The functionality remains unchanged and all tests pass.
Tested also with the new VM 202112022203.
See Kernel-jar.1434
Thanks for your comments, best,
^[^ Jaromir Sent from Squeak Inbox Talk On 2021-12-02T17:37:17-08:00, eliot.miranda at gmail.com wrote: > On Mon, Nov 29, 2021 at 1:46 AM <mail at jaromir.net> wrote: > > > Hi Eliot, all, > > > > > This looks good. The one thing I find concerning is running terminate in > > a different process. Various idioms could legitimately require an unwind > > block to run in the process in which it was created. At the very least > > shouldn't the effectiveProcess be set to be that of the process that is > > terminating while its unwind block executes? > > > > > > For example, what if I did something like this: > > > doWhileMonitoring: aBlock > > > monitoredProcesses add: Processor activeProcess. > > > ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess > > ifAbsent: []] > > > > Would this test prove #terminate works as you'd expect? > > ``` > > | q | > > [ > > [q := Processor activeProcess. > > Processor activeProcess suspend] ensure: [ > > self assert: q = Processor activeProcess] > > ] fork. > > Processor yield. > > q terminate > > ``` > > > > Yes, that captures it. > > > Or with a raised exception: > > ``` > > | q | > > [ > > [q := Processor activeProcess. > > self error] ensure: [ > > self assert: q = Processor activeProcess] > > ] fork. > > Processor yield. > > ``` > > (If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, > > indeed) > > > > Please let me know if this is acceptable. > > > > ish. The reason I thought you weren't setting the effective process is that > it's buried in complete:to:. I'd rather see it up front in > Process>>terminate itself. But I'm happy to defer to you; you've done the > work :-) > > > > > Thanks again, > > > > > > ^[^ Jaromir > > -- > > > > Sent from Squeak Inbox Talk > > > > On 2021-11-28T22:57:25+01:00, mail at jaromir.net wrote: > > > > > Hi Eliot, > > > > > > thanks very much for reviewing this. > > > > > > > This looks good. The one thing I find concerning is running terminate > > in a different process. Various idioms could legitimately require an > > unwind block to run in the process in which it was created. > > > > > > Frankly, I haven't explicitly considered that... In the worst case we > > could revert active process termination bit like it was. Terminating an > > active process via a different process was just supposed to unify (and > > extend) the termination semantics. > > > > > > > At the very least shouldn't the effectiveProcess be set to be that of > > the process that is terminating while its unwind block executes? > > > > > > That's what I though I did in Process>>#complete: topContext to: > > aContext > > > > > > [...] > > > pair := Processor activeProcess > > > evaluate: [topContext > > runUnwindUntilErrorOrReturnFrom: aContext] > > > onBehalfOf: self. > > > [...] > > > > > > where self is the process being terminated. > > #runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being > > terminated and executes the unwind block with effective process set to the > > process being terminated. > > > > > > Or am I mistaken? > > > > > > > For example, what if I did something like this: > > > > doWhileMonitoring: aBlock > > > > monitoredProcesses add: Processor activeProcess. > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > activeProcess ifAbsent: []] > > > > > > > > That unwind blocks are run in the same process as they were created > > seems to me to be a legitimate expectation. Can this be reengineered > > maintaining this invariant, or at least setting the effective process to be > > that of the process being terminated? > > > > > > This is a very interesting example - thanks! Running terminate in a > > different process allows to relatively easily identify all unwind blocks > > half-way through their execution and complete them. In case setting the > > effective process in #complete:to: doesn't work as you requested, then I'm > > afraid unwinding blocks half-way through their execution would be very hard > > to achieve from the process being terminated itself (not impossible though > > - I tried but then doubted it was worth the effort). Or theoretically, it > > might be possible to maintain two stacks for the process being terminated - > > one "original" that needs to be unwound and a second "helper" one running > > terminate - but I haven't gotten beyond the idea, it seemed too unchartered > > :) > > > > > > Besides, when an error happens during unwind, a debugger opens and the > > identity of the process running terminate may change, so the invariant may > > not survive the first error anyway (at least in what I was considering). > > > > > > > So instead of > > > > > > > > self isActiveProcess ifTrue: [ > > > > [self terminate] fork. > > > > ^self suspend]. > > > > > > > > we would use > > > > > > > > self isActiveProcess ifTrue: > > > > [| arnold | > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > newProcess. > > > > arnold resume. > > > > ^self suspend]. > > > > > > > > > > No, that wouldn't work because the Terminator identifies itself as the > > process being terminated when executing [self terminate] resulting in an > > infinite loop. > > > > > > At the moment I can't see why setting the effective process in > > #complete:to: wouldn't suffice but I'll get there :) I'll try to come up > > with a test. > > > > > > I'm intrigued: why did you use '[arnold evaluate:...' and not > > '[Processor activeProcess evaluate:...'; I can't figure out the difference > > :) > > > > > > > I might also consider discarding the return of suspend, because it > > shouldn't be executed normally, and using, say > > > > > > > > ``` > > > > self isActiveProcess ifTrue: > > > > [| arnold | > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > newProcess. > > > > arnold resume. > > > > self suspend. > > > > self error: 'error in terminate; execution resumed after suspend > > for termination']. > > > > ``` > > > > > > No, this wouldn't work if Processor activeProcess terminate was inside > > an unwind block. Terminate would attempt to proceed after self suspend and > > would always raise the error. > > > > > > Thanks again, > > > > > > Jaromir > > > > > > PS: I used this example to test your suggestions: > > > > > > ``` > > > | p | > > > p := [ > > > [ > > > [ ] ensure: [ > > > [Processor activeProcess terminate] ensure: [ > > > Transcript show: 'x1']. > > > Transcript show: 'x2'] > > > ] ensure: [ > > > Transcript show: 'x3']. > > > Transcript show: 'x4' > > > ] newProcess. > > > p resume. > > > "Two yields necessary: terminate active is a two-step procedure" > > > Processor yield. Processor yield. > > > Transcript show: p isTerminated printString > > > > > > "prints x1 x2 x3" > > > ``` > > > > > > > > > ^[^ Jaromir > > > -- > > > > > > Sent from Squeak Inbox Talk > > > > > > On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote: > > > > > > > Hi Jaromir, Hi Christoph, > > > > > > > > On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote: > > > > > > > > > Hi all, Christoph, > > > > > > > > > > I finally managed to separate #terminate and the 'block cannot return > > > > > issue' so that we can discuss them independently :) And thanks to > > Eliot's > > > > > solution of the infinite recursion in #doesNotUnderstand the > > #terminate > > > > > code is now cleaner. I haven't included Eliot's modification of > > > > > #doesNotUnderstand in my changeset but paste it below instead > > (because I'm > > > > > not the author; I hope it'll be merged though): > > > > > > > > > > The #terminate code presented in Kernel-jar.1426 is the final > > version of > > > > > my effort to get rid of all the bugs reported in this thread (and > > make > > > > > #terminate semantics more consistent). It passes all standard tests > > and the > > > > > tests complementing this changeset: > > > > > > > > > > KernelTests-jar.406 (Terminator test) > > > > > KernelTests-jar.407 (McClure test) > > > > > Tests-jar.466 (unwind tests) > > > > > ToolsTests-jar.105 (debugger tests) > > > > > > > > > > In addition, it's fully compatible with Christoph's solution of the > > 'block > > > > > cannot return issue' presented in Kernel-ct.1405 ( > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html > > ). > > > > > > > > > > > > > > > Summary and discussion about the bugs and changes in #terminate: > > > > > > > http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal-td5128285.html > > > > > > > > > > I think it's ready for merging... Please review. > > > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > in a > > > > different process. Various idioms could legitimately require an unwind > > > > block to run in the process in which it was created. At the very least > > > > shouldn't the effectiveProcess be set to be that of the process that is > > > > terminating while its unwind block executes? > > > > > > > > For example, what if I did something like this: > > > > doWhileMonitoring: aBlock > > > > monitoredProcesses add: Processor activeProcess. > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > activeProcess > > > > ifAbsent: []] > > > > > > > > That unwind blocks are run in the same process as they were created > > seems > > > > to me to be a legitimate expectation. Can this be reengineered > > maintaining > > > > this invariant, or at least setting the effective process to be that > > of the > > > > process being terminated? > > > > > > > > So instead of > > > > > > > > self isActiveProcess ifTrue: [ > > > > [self terminate] fork. > > > > ^self suspend]. > > > > > > > > we would use > > > > > > > > self isActiveProcess ifTrue: > > > > [| arnold | > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > newProcess. > > > > arnold resume. > > > > ^self suspend]. > > > > > > > > I might also consider discarding the return of suspend, because it > > > > shouldn't be executed normally, and using, say > > > > > > > > self isActiveProcess ifTrue: > > > > [| arnold | > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > newProcess. > > > > arnold resume. > > > > self suspend. > > > > self error: 'error in terminate; execution resumed after > > suspend > > > > for termination']. > > > > > > > > > > > > > -- > > > > > Eliot's fix of infinite recursion in doesNotUnderstand: > > > > > > > > > > > > > > > doesNotUnderstand: aMessage > > > > > "Handle the fact that there was an attempt to send the given > > > > > message to the receiver but the receiver does not > > understand > > > > > this message (typically sent from the machine when a > > message > > > > > is sent to the receiver and no method is defined for that > > > > > selector)." > > > > > > > > > > "Testing: (3 activeProcess)" > > > > > > > > > > | exception resumeValue | > > > > > (exception := MessageNotUnderstood new) > > > > > message: aMessage; > > > > > receiver: self. > > > > > resumeValue := exception signal. > > > > > ^exception reachedDefaultHandler > > > > > ifFalse: [resumeValue] > > > > > "----------> this wrapper detecting recursion is added: > > -------------->" > > > > > > > > > > ifTrue: [ > > > > > [aMessage sentTo: self] > > > > > on: MessageNotUnderstood > > > > > do: [:ex| | args | > > > > > args := ex message arguments. > > > > > (ex receiver == self > > > > > and: [ex message selector == > > > > > aMessage selector > > > > > and: [(1 to: aMessage > > numArgs) > > > > > allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) > > ifFalse: > > > > > [ex pass]. > > > > > self error: 'infinite recursion > > in > > > > > doesNotUnderstand:']] > > > > > > > > > > (see discussion > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.html > > > > > ) > > > > > > > > > > best, > > > > > > > > > > ^[^ Jaromir > > > > > -- > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote: > > > > > > > > > > > Hi Christoph, > > > > > > > > > > > > Once more, apologies for taking so long to respond. > > > > > > > > > > > > > > [...] there really are two distinct unwind semantics : one > > "light" > > > > > for regular returns and one "heavy" for termination. Both are very > > similar > > > > > yet each require a slightly different behavior - that's why the > > duality > > > > > #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or > > #complete: > > > > > / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > extra > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > code > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > Yes, I understand your concern and agree two ifs would allow to > > pack the > > > > > two twin beasts into one can (I refer to "rueorf" & "ruueorf"). > > Also, we > > > > > could consider Process >> complete:to: as a generalized version of > > Process > > > > > >> complete and rewrite it accordingly. This is doable, maybe even > > > > > desirable but for the time being I'd very much prefer to keep them > > separate > > > > > to facilitate troubleshooting of potential problems (or localizing > > them). > > > > > Each of the two similar methods belong to a different execution path > > after > > > > > all. And besides, #runUntilErrorOrReturnFrom is already crazy enough > > and I > > > > > don't feel like making it even more complex now :) > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > wondering whether it wouldn't have the same unwind problem with > > non-local > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > described in > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > I put #unwindTo: aside for the moment; it contains the same flaw > > and > > > > > deserves its own thread. > > > > > > > > > > > > > Unfortunately I've lost track of these infinite loops [...] > > > > > > > > > > > > Yeah, tell me about it :) > > > > > > > > > > > > > [...]- could you maybe point me to some concrete examples that > > lead to > > > > > an infinite recursion without this special check? :-) > > > > > > > > > > > > Try: > > > > > > > > > > > > ``` > > > > > > [self error: 'error'] ensure: [self gotcha. Transcript show: 'all > > > > > cleaned up! '] > > > > > > ``` > > > > > > > > > > > > 'gotcha' represents a method unknown to the system. The code will > > print > > > > > 'all cleaned up! ' after abandoning the error and the debugger - > > meaning > > > > > the system will survive this code and execute all cleanups (aka > > unwinds). > > > > > In case you use your #terminateAggressively as the default action for > > > > > abandoning the Debugger, you won't have any problem of course > > because you > > > > > skip completing the halfway through unwind. What I'm saying is: even > > if you > > > > > chose the classic heavy #terminate to exit the Debugger, the code > > will > > > > > execute remaining unwinds as one might expect (or hope for). > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > Are you > > > > > maybe missing something like "cxt terminate" in the "No error was > > raised" > > > > > case? Just wondering. > > > > > > > > > > > > I guess I know what you mean... 'ctxt' would get garbage collected > > soon > > > > > anyway so I left it as is to keep the code as simple as possible. > > > > > > > > > > > > > > > > > > > > > > > > Christoph, I'll also address your comments regarding the > > > > > BlockCannotReturn behavior here: > > > > > > > > > > > > > Isn't that ProceedBlockCannotReturn tautologous? I think that by > > > > > actively proceeding from a BlockCannotReturn error users already > > accept > > > > > that they are going to resume execution in another way. > > > > > > > > > > > > Well, the very proceeding from a BlockCannotReturn error sort of > > > > > violates common sense but during our lengthy discussion you > > convinced me it > > > > > makes a very good sense when troubleshooting :) The idea is by no > > means > > > > > trivial - unlike hitting Proceed :) So an extra warning can't hurt... > > > > > > > > > > > > But more importantly, I need something to know the user let the > > process > > > > > continue after reaching the BlockCannotReturn error - thus the new > > > > > ProceedBlockCannotReturn exception which allows Process >> > > #complete:to: to > > > > > deal with the new course of events. > > > > > > > > > > > > > Apart from that, the message text of your new warning is not > > correct > > > > > if self pc <= self endPC. :-) > > > > > > > > > > > > Yes, and I'd like to make the warning message more verbose so even > > if > > > > > someone hit Proceed without much thinking they could get an idea > > what's > > > > > about to happen :) Should the warning interfere with some potential > > > > > automation efforts we could come up with some alternative way. > > > > > > > > > > > > I'll copy my reply regarding BCR to the "The Inbox: > > Kernel-ct.1405.mcz" > > > > > thread and we can continue discussing this particular issue there. > > It's > > > > > really a separate issue and I included your patch here along with > > the main > > > > > termination code because it perfectly complements it and prevents the > > > > > disastrous crashes caused by Proceeding the BCR error. > > > > > > > > > > > > > [...] the computation terminated is also wrong, IMO, you should > > get a > > > > > BlockCannotReturn here. > > > > > > > > > > > > Yes, I agree, there's definitely more to it which deserves to be > > > > > discussed thoroughly and separately in the "The Inbox: > > Kernel-ct.1405.mcz" > > > > > thread. > > > > > > > > > > > > If you agree I'd really appreciate if your fix could be accepted > > here in > > > > > including the ProceedBlockCannotReturn exception I need to make it > > work > > > > > together with the code I'm presenting here. If we come up with a > > better > > > > > idea in the other discussion we can make amends here as well; I take > > it as > > > > > a patch, not a definitive solution :) > > > > > > > > > > > > Thanks again so much for all your suggestions and examples! It's > > always > > > > > a pleasure :) Plus - the Inbox Talk is priceless... I use it > > exclusively > > > > > now and look forward to any improvements you might come up in the > > future ;) > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > -- > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > On 2021-08-22T16:49:10+02:00, christoph.thiede at > > > > > student.hpi.uni-potsdam.de wrote: > > > > > > > > > > > > > Hi Jaromir, > > > > > > > > > > > > > > > Yes, I was wondering why I couldn't get rid of the duplication > > and > > > > > now I think it's because there really are two distinct unwind > > semantics : > > > > > one "light" for regular returns and one "heavy" for termination. > > Both are > > > > > very similar yet each require a slightly different behavior - that's > > why > > > > > the duality #runUntilErrorOrReturnFrom / > > #runUnwindUntilErrorOrReturnFrom > > > > > or #complete: / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > extra > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > code > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > wondering whether it wouldn't have the same unwind problem with > > non-local > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > described in > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > > But in general - yes, any method/exception purposefully (or > > not) > > > > > written to create a loop will break this patch (I admit it is just a > > patch > > > > > really). I extracted it to #complete:to: to make #terminate clean; > > this is > > > > > a WIP; I wish there was a holistic solution to this - maybe checking > > for > > > > > exception recursion by default? :) > > > > > > > > > > > > > > Sounds better already, if feasible! But how would you detect > > this? > > > > > Unfortunately I've lost track of these infinite loops - could you > > maybe > > > > > point me to some concrete examples that lead to an infinite recursion > > > > > without this special check? :-) > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > Are you > > > > > maybe missing something like "cxt terminate" in the "No error was > > raised" > > > > > case? Just wondering. > > > > > > > > > > > > > > Best, > > > > > > > Christoph > > > > > > > > > > > > > > [1] > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > --- > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote: > > > > > > > > > > > > > > > Jaromir Matas wrote > > > > > > > > > Hi All, > > > > > > > > > I've sent an updated version of #teminate integrating > > Christoph's > > > > > solution > > > > > > > > > of BlockCannotReturn recursion problem (in [1]), along with a > > > > > battery of > > > > > > > > > tests exploring termination of nested ensure and cascading > > errors > > > > > behavior > > > > > > > > > (Debugger tests are for info and a final version can wait > > until > > > > > releasing > > > > > > > > > Christoph's proposal in [2]). > > > > > > > > > > > > > > > > > > It's pretty much final, I hope... > > > > > > > > > > > > > > > > > > Any complaints about #terminate - please shout ;) > > > > > > > > > > > > > > > > > > [1] > > > > > http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html > > > > > > > > > [2] > > > > > > > > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > Here's the link: > > > > > > > > > > http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- > > > > > > > > ^[^ Jaromir > > > > > > > > -- > > > > > > > > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > > > > > > > > > > > > > > > > > > > > > > >
Hi Marcel, Christoph and all,
Let me summarize the current state of the #terminate fix (Kernel-jar.1437):
There's been a development in the issue recently; two more bugs have surfaced and even if they are solvable independently of the latest #terminate fix I chose to include them in the #teminate changeset to, hopefully, facilitate the integration.
So the changeset Kernel-jar.1437 addresses all of:
(1) the original termination issues from [1] (2) multiple termination (/resumption) issue from [2] (3) #releaseCriticalSection issue from [3]
In case you decide to integrate fixes for (2) and (3) independently I can adjust #terminate accordingly following those fixes.
In addition there are two more related issues (but again, independent of the latest #terminate):
(4) stepOver bug solution discussed in [4] and solved in Kernel-jar.1415 (5) proceedable #cannotReturn extension by Christoph discussed in [5] and presented in Kernel-ct.1405
I haven't included these two in #terminate because the solutions don't interact with #terminate and in case of (5) Christoph's the author.
Here's *my opinion* about the status of each: (1) ready (2) proposed solution works but may require some (cosmetic) polishing (3) ready (4) ready (5) I like it
The following tests in the Inbox are still relevant:
KernelTests-jar.420 (releaseCriticalSection bug) KernelTests-jar.417 (multiple termination / resumption) KernelTests-jar.416 (ensure as top) KernelTests-jar.415 (unwind tests) KernelTests-jar.406 (nested termination) ToolsTests-jar.109 (debugger termination)
To cleanup the Inbox, the following changesets can be moved to Treated:
Tests-jar.466 (unwind tests - superseded) Kernel-jar.1435 (#terminate - superseded) Kernel-jar.1426 (#terminate - superseded) Kernel-jar.1413 (stepOver bug - superseded) Kernel-jar.1421 (stepOver bug - superseded) KernelTests-jar.419 (releaseCriticalSection bug - superseded) KernelTests-jar.407 (McClure test - withdrawing) ToolsTests-jar.105 (debugger tests - superseded) KernelTests-jar.393 (#priority bug - superseded)
[1] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-April/214633.htm... [2] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-December/217695.... [3] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-December/217744.... [4] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-April/214987.htm... [5] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html
Christoph, do you want me to update the individual discussions with the most up-to-date status or will you just to plough through them? :)
Thanks, best regards,
~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-12-12T15:49:48+01:00, mail@jaromir.net wrote:
Hi All, Christoph, Eliot,
One more update of new #terminate; I'm sorry for further extending this thread. I've rediscovered how very tricky debugging the BCR is; take this example with nested ensure blocks containg non-local returns:
[[[] ensure: [^2]] ensure: [^42]] fork
#terminate needs to avoid returning from the two dangerous contexts while still unwinding as much as possible without a crash :) Also, there's one ensure block inside the debugger opening sequence (inside Debugger class>>openOn:context:... ) which needs to be executed when abandoning the debugger. (I always forget the debugger opening sequence on the top of the stack of the process being debugged is hidden by the debugger)
I hope the latest #terminate in Kernel-jar.1435 deals with all these issues correctly at last (on top off all other issues and bugs).
Many thanks for your comments or a review.
best,
^[^ Jaromir Sent from Squeak Inbox Talk On 2021-12-10T13:45:26+01:00, mail at jaromir.net wrote: > Hi Eliot, Christoph, all > > Thanks to Eliot's remarks I substantially simplified #terminate's helper method #runUnwind:onBehalfOf: (former #complete:to:) and updated comments. The functionality remains unchanged and all tests pass. > > Tested also with the new VM 202112022203. > > See Kernel-jar.1434 > > Thanks for your comments, > best, > > ~~~ > ^[^ Jaromir > > Sent from Squeak Inbox Talk > > On 2021-12-02T17:37:17-08:00, eliot.miranda at gmail.com wrote: > > > On Mon, Nov 29, 2021 at 1:46 AM <mail at jaromir.net> wrote: > > > > > Hi Eliot, all, > > > > > > > This looks good. The one thing I find concerning is running terminate in > > > a different process. Various idioms could legitimately require an unwind > > > block to run in the process in which it was created. At the very least > > > shouldn't the effectiveProcess be set to be that of the process that is > > > terminating while its unwind block executes? > > > > > > > > For example, what if I did something like this: > > > > doWhileMonitoring: aBlock > > > > monitoredProcesses add: Processor activeProcess. > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess > > > ifAbsent: []] > > > > > > Would this test prove #terminate works as you'd expect? > > > ``` > > > | q | > > > [ > > > [q := Processor activeProcess. > > > Processor activeProcess suspend] ensure: [ > > > self assert: q = Processor activeProcess] > > > ] fork. > > > Processor yield. > > > q terminate > > > ``` > > > > > > > Yes, that captures it. > > > > > Or with a raised exception: > > > ``` > > > | q | > > > [ > > > [q := Processor activeProcess. > > > self error] ensure: [ > > > self assert: q = Processor activeProcess] > > > ] fork. > > > Processor yield. > > > ``` > > > (If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, > > > indeed) > > > > > > Please let me know if this is acceptable. > > > > > > > ish. The reason I thought you weren't setting the effective process is that > > it's buried in complete:to:. I'd rather see it up front in > > Process>>terminate itself. But I'm happy to defer to you; you've done the > > work :-) > > > > > > > > > Thanks again, > > > > > > > > > > ^[^ Jaromir > > > -- > > > > > > Sent from Squeak Inbox Talk > > > > > > On 2021-11-28T22:57:25+01:00, mail at jaromir.net wrote: > > > > > > > Hi Eliot, > > > > > > > > thanks very much for reviewing this. > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > > in a different process. Various idioms could legitimately require an > > > unwind block to run in the process in which it was created. > > > > > > > > Frankly, I haven't explicitly considered that... In the worst case we > > > could revert active process termination bit like it was. Terminating an > > > active process via a different process was just supposed to unify (and > > > extend) the termination semantics. > > > > > > > > > At the very least shouldn't the effectiveProcess be set to be that of > > > the process that is terminating while its unwind block executes? > > > > > > > > That's what I though I did in Process>>#complete: topContext to: > > > aContext > > > > > > > > [...] > > > > pair := Processor activeProcess > > > > evaluate: [topContext > > > runUnwindUntilErrorOrReturnFrom: aContext] > > > > onBehalfOf: self. > > > > [...] > > > > > > > > where self is the process being terminated. > > > #runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being > > > terminated and executes the unwind block with effective process set to the > > > process being terminated. > > > > > > > > Or am I mistaken? > > > > > > > > > For example, what if I did something like this: > > > > > doWhileMonitoring: aBlock > > > > > monitoredProcesses add: Processor activeProcess. > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > > activeProcess ifAbsent: []] > > > > > > > > > > That unwind blocks are run in the same process as they were created > > > seems to me to be a legitimate expectation. Can this be reengineered > > > maintaining this invariant, or at least setting the effective process to be > > > that of the process being terminated? > > > > > > > > This is a very interesting example - thanks! Running terminate in a > > > different process allows to relatively easily identify all unwind blocks > > > half-way through their execution and complete them. In case setting the > > > effective process in #complete:to: doesn't work as you requested, then I'm > > > afraid unwinding blocks half-way through their execution would be very hard > > > to achieve from the process being terminated itself (not impossible though > > > - I tried but then doubted it was worth the effort). Or theoretically, it > > > might be possible to maintain two stacks for the process being terminated - > > > one "original" that needs to be unwound and a second "helper" one running > > > terminate - but I haven't gotten beyond the idea, it seemed too unchartered > > > :) > > > > > > > > Besides, when an error happens during unwind, a debugger opens and the > > > identity of the process running terminate may change, so the invariant may > > > not survive the first error anyway (at least in what I was considering). > > > > > > > > > So instead of > > > > > > > > > > self isActiveProcess ifTrue: [ > > > > > [self terminate] fork. > > > > > ^self suspend]. > > > > > > > > > > we would use > > > > > > > > > > self isActiveProcess ifTrue: > > > > > [| arnold | > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > newProcess. > > > > > arnold resume. > > > > > ^self suspend]. > > > > > > > > > > > > > No, that wouldn't work because the Terminator identifies itself as the > > > process being terminated when executing [self terminate] resulting in an > > > infinite loop. > > > > > > > > At the moment I can't see why setting the effective process in > > > #complete:to: wouldn't suffice but I'll get there :) I'll try to come up > > > with a test. > > > > > > > > I'm intrigued: why did you use '[arnold evaluate:...' and not > > > '[Processor activeProcess evaluate:...'; I can't figure out the difference > > > :) > > > > > > > > > I might also consider discarding the return of suspend, because it > > > shouldn't be executed normally, and using, say > > > > > > > > > > ``` > > > > > self isActiveProcess ifTrue: > > > > > [| arnold | > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > newProcess. > > > > > arnold resume. > > > > > self suspend. > > > > > self error: 'error in terminate; execution resumed after suspend > > > for termination']. > > > > > ``` > > > > > > > > No, this wouldn't work if Processor activeProcess terminate was inside > > > an unwind block. Terminate would attempt to proceed after self suspend and > > > would always raise the error. > > > > > > > > Thanks again, > > > > > > > > Jaromir > > > > > > > > PS: I used this example to test your suggestions: > > > > > > > > ``` > > > > | p | > > > > p := [ > > > > [ > > > > [ ] ensure: [ > > > > [Processor activeProcess terminate] ensure: [ > > > > Transcript show: 'x1']. > > > > Transcript show: 'x2'] > > > > ] ensure: [ > > > > Transcript show: 'x3']. > > > > Transcript show: 'x4' > > > > ] newProcess. > > > > p resume. > > > > "Two yields necessary: terminate active is a two-step procedure" > > > > Processor yield. Processor yield. > > > > Transcript show: p isTerminated printString > > > > > > > > "prints x1 x2 x3" > > > > ``` > > > > > > > > > > > > ^[^ Jaromir > > > > -- > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote: > > > > > > > > > Hi Jaromir, Hi Christoph, > > > > > > > > > > On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote: > > > > > > > > > > > Hi all, Christoph, > > > > > > > > > > > > I finally managed to separate #terminate and the 'block cannot return > > > > > > issue' so that we can discuss them independently :) And thanks to > > > Eliot's > > > > > > solution of the infinite recursion in #doesNotUnderstand the > > > #terminate > > > > > > code is now cleaner. I haven't included Eliot's modification of > > > > > > #doesNotUnderstand in my changeset but paste it below instead > > > (because I'm > > > > > > not the author; I hope it'll be merged though): > > > > > > > > > > > > The #terminate code presented in Kernel-jar.1426 is the final > > > version of > > > > > > my effort to get rid of all the bugs reported in this thread (and > > > make > > > > > > #terminate semantics more consistent). It passes all standard tests > > > and the > > > > > > tests complementing this changeset: > > > > > > > > > > > > KernelTests-jar.406 (Terminator test) > > > > > > KernelTests-jar.407 (McClure test) > > > > > > Tests-jar.466 (unwind tests) > > > > > > ToolsTests-jar.105 (debugger tests) > > > > > > > > > > > > In addition, it's fully compatible with Christoph's solution of the > > > 'block > > > > > > cannot return issue' presented in Kernel-ct.1405 ( > > > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html > > > ). > > > > > > > > > > > > > > > > > > Summary and discussion about the bugs and changes in #terminate: > > > > > > > > > http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal-td5128285.html > > > > > > > > > > > > I think it's ready for merging... Please review. > > > > > > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > > in a > > > > > different process. Various idioms could legitimately require an unwind > > > > > block to run in the process in which it was created. At the very least > > > > > shouldn't the effectiveProcess be set to be that of the process that is > > > > > terminating while its unwind block executes? > > > > > > > > > > For example, what if I did something like this: > > > > > doWhileMonitoring: aBlock > > > > > monitoredProcesses add: Processor activeProcess. > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > > activeProcess > > > > > ifAbsent: []] > > > > > > > > > > That unwind blocks are run in the same process as they were created > > > seems > > > > > to me to be a legitimate expectation. Can this be reengineered > > > maintaining > > > > > this invariant, or at least setting the effective process to be that > > > of the > > > > > process being terminated? > > > > > > > > > > So instead of > > > > > > > > > > self isActiveProcess ifTrue: [ > > > > > [self terminate] fork. > > > > > ^self suspend]. > > > > > > > > > > we would use > > > > > > > > > > self isActiveProcess ifTrue: > > > > > [| arnold | > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > newProcess. > > > > > arnold resume. > > > > > ^self suspend]. > > > > > > > > > > I might also consider discarding the return of suspend, because it > > > > > shouldn't be executed normally, and using, say > > > > > > > > > > self isActiveProcess ifTrue: > > > > > [| arnold | > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > newProcess. > > > > > arnold resume. > > > > > self suspend. > > > > > self error: 'error in terminate; execution resumed after > > > suspend > > > > > for termination']. > > > > > > > > > > > > > > > > -- > > > > > > Eliot's fix of infinite recursion in doesNotUnderstand: > > > > > > > > > > > > > > > > > > doesNotUnderstand: aMessage > > > > > > "Handle the fact that there was an attempt to send the given > > > > > > message to the receiver but the receiver does not > > > understand > > > > > > this message (typically sent from the machine when a > > > message > > > > > > is sent to the receiver and no method is defined for that > > > > > > selector)." > > > > > > > > > > > > "Testing: (3 activeProcess)" > > > > > > > > > > > > | exception resumeValue | > > > > > > (exception := MessageNotUnderstood new) > > > > > > message: aMessage; > > > > > > receiver: self. > > > > > > resumeValue := exception signal. > > > > > > ^exception reachedDefaultHandler > > > > > > ifFalse: [resumeValue] > > > > > > "----------> this wrapper detecting recursion is added: > > > -------------->" > > > > > > > > > > > > ifTrue: [ > > > > > > [aMessage sentTo: self] > > > > > > on: MessageNotUnderstood > > > > > > do: [:ex| | args | > > > > > > args := ex message arguments. > > > > > > (ex receiver == self > > > > > > and: [ex message selector == > > > > > > aMessage selector > > > > > > and: [(1 to: aMessage > > > numArgs) > > > > > > allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) > > > ifFalse: > > > > > > [ex pass]. > > > > > > self error: 'infinite recursion > > > in > > > > > > doesNotUnderstand:']] > > > > > > > > > > > > (see discussion > > > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.html > > > > > > ) > > > > > > > > > > > > best, > > > > > > > > > > > > ^[^ Jaromir > > > > > > -- > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote: > > > > > > > > > > > > > Hi Christoph, > > > > > > > > > > > > > > Once more, apologies for taking so long to respond. > > > > > > > > > > > > > > > > [...] there really are two distinct unwind semantics : one > > > "light" > > > > > > for regular returns and one "heavy" for termination. Both are very > > > similar > > > > > > yet each require a slightly different behavior - that's why the > > > duality > > > > > > #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or > > > #complete: > > > > > > / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > > extra > > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > > code > > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > Yes, I understand your concern and agree two ifs would allow to > > > pack the > > > > > > two twin beasts into one can (I refer to "rueorf" & "ruueorf"). > > > Also, we > > > > > > could consider Process >> complete:to: as a generalized version of > > > Process > > > > > > >> complete and rewrite it accordingly. This is doable, maybe even > > > > > > desirable but for the time being I'd very much prefer to keep them > > > separate > > > > > > to facilitate troubleshooting of potential problems (or localizing > > > them). > > > > > > Each of the two similar methods belong to a different execution path > > > after > > > > > > all. And besides, #runUntilErrorOrReturnFrom is already crazy enough > > > and I > > > > > > don't feel like making it even more complex now :) > > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > > wondering whether it wouldn't have the same unwind problem with > > > non-local > > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > > described in > > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > I put #unwindTo: aside for the moment; it contains the same flaw > > > and > > > > > > deserves its own thread. > > > > > > > > > > > > > > > Unfortunately I've lost track of these infinite loops [...] > > > > > > > > > > > > > > Yeah, tell me about it :) > > > > > > > > > > > > > > > [...]- could you maybe point me to some concrete examples that > > > lead to > > > > > > an infinite recursion without this special check? :-) > > > > > > > > > > > > > > Try: > > > > > > > > > > > > > > ``` > > > > > > > [self error: 'error'] ensure: [self gotcha. Transcript show: 'all > > > > > > cleaned up! '] > > > > > > > ``` > > > > > > > > > > > > > > 'gotcha' represents a method unknown to the system. The code will > > > print > > > > > > 'all cleaned up! ' after abandoning the error and the debugger - > > > meaning > > > > > > the system will survive this code and execute all cleanups (aka > > > unwinds). > > > > > > In case you use your #terminateAggressively as the default action for > > > > > > abandoning the Debugger, you won't have any problem of course > > > because you > > > > > > skip completing the halfway through unwind. What I'm saying is: even > > > if you > > > > > > chose the classic heavy #terminate to exit the Debugger, the code > > > will > > > > > > execute remaining unwinds as one might expect (or hope for). > > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > > Are you > > > > > > maybe missing something like "cxt terminate" in the "No error was > > > raised" > > > > > > case? Just wondering. > > > > > > > > > > > > > > I guess I know what you mean... 'ctxt' would get garbage collected > > > soon > > > > > > anyway so I left it as is to keep the code as simple as possible. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Christoph, I'll also address your comments regarding the > > > > > > BlockCannotReturn behavior here: > > > > > > > > > > > > > > > Isn't that ProceedBlockCannotReturn tautologous? I think that by > > > > > > actively proceeding from a BlockCannotReturn error users already > > > accept > > > > > > that they are going to resume execution in another way. > > > > > > > > > > > > > > Well, the very proceeding from a BlockCannotReturn error sort of > > > > > > violates common sense but during our lengthy discussion you > > > convinced me it > > > > > > makes a very good sense when troubleshooting :) The idea is by no > > > means > > > > > > trivial - unlike hitting Proceed :) So an extra warning can't hurt... > > > > > > > > > > > > > > But more importantly, I need something to know the user let the > > > process > > > > > > continue after reaching the BlockCannotReturn error - thus the new > > > > > > ProceedBlockCannotReturn exception which allows Process >> > > > #complete:to: to > > > > > > deal with the new course of events. > > > > > > > > > > > > > > > Apart from that, the message text of your new warning is not > > > correct > > > > > > if self pc <= self endPC. :-) > > > > > > > > > > > > > > Yes, and I'd like to make the warning message more verbose so even > > > if > > > > > > someone hit Proceed without much thinking they could get an idea > > > what's > > > > > > about to happen :) Should the warning interfere with some potential > > > > > > automation efforts we could come up with some alternative way. > > > > > > > > > > > > > > I'll copy my reply regarding BCR to the "The Inbox: > > > Kernel-ct.1405.mcz" > > > > > > thread and we can continue discussing this particular issue there. > > > It's > > > > > > really a separate issue and I included your patch here along with > > > the main > > > > > > termination code because it perfectly complements it and prevents the > > > > > > disastrous crashes caused by Proceeding the BCR error. > > > > > > > > > > > > > > > [...] the computation terminated is also wrong, IMO, you should > > > get a > > > > > > BlockCannotReturn here. > > > > > > > > > > > > > > Yes, I agree, there's definitely more to it which deserves to be > > > > > > discussed thoroughly and separately in the "The Inbox: > > > Kernel-ct.1405.mcz" > > > > > > thread. > > > > > > > > > > > > > > If you agree I'd really appreciate if your fix could be accepted > > > here in > > > > > > including the ProceedBlockCannotReturn exception I need to make it > > > work > > > > > > together with the code I'm presenting here. If we come up with a > > > better > > > > > > idea in the other discussion we can make amends here as well; I take > > > it as > > > > > > a patch, not a definitive solution :) > > > > > > > > > > > > > > Thanks again so much for all your suggestions and examples! It's > > > always > > > > > > a pleasure :) Plus - the Inbox Talk is priceless... I use it > > > exclusively > > > > > > now and look forward to any improvements you might come up in the > > > future ;) > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > > -- > > > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > On 2021-08-22T16:49:10+02:00, christoph.thiede at > > > > > > student.hpi.uni-potsdam.de wrote: > > > > > > > > > > > > > > > Hi Jaromir, > > > > > > > > > > > > > > > > > Yes, I was wondering why I couldn't get rid of the duplication > > > and > > > > > > now I think it's because there really are two distinct unwind > > > semantics : > > > > > > one "light" for regular returns and one "heavy" for termination. > > > Both are > > > > > > very similar yet each require a slightly different behavior - that's > > > why > > > > > > the duality #runUntilErrorOrReturnFrom / > > > #runUnwindUntilErrorOrReturnFrom > > > > > > or #complete: / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > > extra > > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > > code > > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > > wondering whether it wouldn't have the same unwind problem with > > > non-local > > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > > described in > > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > > > > But in general - yes, any method/exception purposefully (or > > > not) > > > > > > written to create a loop will break this patch (I admit it is just a > > > patch > > > > > > really). I extracted it to #complete:to: to make #terminate clean; > > > this is > > > > > > a WIP; I wish there was a holistic solution to this - maybe checking > > > for > > > > > > exception recursion by default? :) > > > > > > > > > > > > > > > > Sounds better already, if feasible! But how would you detect > > > this? > > > > > > Unfortunately I've lost track of these infinite loops - could you > > > maybe > > > > > > point me to some concrete examples that lead to an infinite recursion > > > > > > without this special check? :-) > > > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > > Are you > > > > > > maybe missing something like "cxt terminate" in the "No error was > > > raised" > > > > > > case? Just wondering. > > > > > > > > > > > > > > > > Best, > > > > > > > > Christoph > > > > > > > > > > > > > > > > [1] > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > --- > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > > > On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote: > > > > > > > > > > > > > > > > > Jaromir Matas wrote > > > > > > > > > > Hi All, > > > > > > > > > > I've sent an updated version of #teminate integrating > > > Christoph's > > > > > > solution > > > > > > > > > > of BlockCannotReturn recursion problem (in [1]), along with a > > > > > > battery of > > > > > > > > > > tests exploring termination of nested ensure and cascading > > > errors > > > > > > behavior > > > > > > > > > > (Debugger tests are for info and a final version can wait > > > until > > > > > > releasing > > > > > > > > > > Christoph's proposal in [2]). > > > > > > > > > > > > > > > > > > > > It's pretty much final, I hope... > > > > > > > > > > > > > > > > > > > > Any complaints about #terminate - please shout ;) > > > > > > > > > > > > > > > > > > > > [1] > > > > > > http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > Here's the link: > > > > > > > > > > > > http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- > > > > > > > > > ^[^ Jaromir > > > > > > > > > -- > > > > > > > > > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > > > > > > > > > > > > > > > > > > > > > > > > > >
Hi Eliot, all,
I hear your concerns about running terminate in a different process.
So for me, KISS. Just support something that guards against multiple termination, and in the debugger, reveals clearly where a process is in the course of running unwinds.
And I really *don'* like the idea of running terminate in a separate process. This has bad performance implications. Most terminations can be done in the process itself; that should be the expectation.
Agreed, I didn't like it either.
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant?
To my surprise the solution appears to be quite straightforward: for a terminating process, create a parallel stack and run the process's own unwinds there; all tests pass and I'm very anxious to hear your opinion about possible drawbacks of one process executing on two stacks.
#terminate then reads:
``` terminate
| top | "If terminating the active process, create a parallel stack and run unwinds from there." self isActiveProcess ifTrue: [ top := thisContext. ^[self unwind: top; suspend] asContext jump].
[] ensure: [ suspendedContext ifNil: [^self error: 'Process already terminated or terminating']. Smalltalk processSuspensionUnblocks ifFalse: [ "this part is for revised resume semantics introduced in 2022 VMs" self suspend. top := suspendedContext. suspendedContext := nil. top releaseCriticalSection. self unwind: top] ifTrue: [ "this part is for backward compatibilty with pre-2022 VMs" | oldList | oldList := self suspend. self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]). top := suspendedContext. suspendedContext := nil. self unwind: top]] ```
The whole version including comments and the helper methods #unwind: and #unwind:to: is in Kernel-jar.1443.
I've also included a simplified version of #releaseCriticalSection (which is now under Context).
This version of #terminate is backward compatible with older VMs (without the revised suspend).
A battery of updated tests is in KernelTests-jar.421.
Thanks for any comments.
best, ~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk
On 2021-12-22T18:22:50+01:00, mail@jaromir.net wrote:
Hi Marcel, Christoph and all,
Let me summarize the current state of the #terminate fix (Kernel-jar.1437):
There's been a development in the issue recently; two more bugs have surfaced and even if they are solvable independently of the latest #terminate fix I chose to include them in the #teminate changeset to, hopefully, facilitate the integration.
So the changeset Kernel-jar.1437 addresses all of:
(1) the original termination issues from [1] (2) multiple termination (/resumption) issue from [2] (3) #releaseCriticalSection issue from [3]
In case you decide to integrate fixes for (2) and (3) independently I can adjust #terminate accordingly following those fixes.
In addition there are two more related issues (but again, independent of the latest #terminate):
(4) stepOver bug solution discussed in [4] and solved in Kernel-jar.1415 (5) proceedable #cannotReturn extension by Christoph discussed in [5] and presented in Kernel-ct.1405
I haven't included these two in #terminate because the solutions don't interact with #terminate and in case of (5) Christoph's the author.
Here's *my opinion* about the status of each: (1) ready (2) proposed solution works but may require some (cosmetic) polishing (3) ready (4) ready (5) I like it
The following tests in the Inbox are still relevant:
KernelTests-jar.420 (releaseCriticalSection bug) KernelTests-jar.417 (multiple termination / resumption) KernelTests-jar.416 (ensure as top) KernelTests-jar.415 (unwind tests) KernelTests-jar.406 (nested termination) ToolsTests-jar.109 (debugger termination)
To cleanup the Inbox, the following changesets can be moved to Treated:
Tests-jar.466 (unwind tests - superseded) Kernel-jar.1435 (#terminate - superseded) Kernel-jar.1426 (#terminate - superseded) Kernel-jar.1413 (stepOver bug - superseded) Kernel-jar.1421 (stepOver bug - superseded) KernelTests-jar.419 (releaseCriticalSection bug - superseded) KernelTests-jar.407 (McClure test - withdrawing) ToolsTests-jar.105 (debugger tests - superseded) KernelTests-jar.393 (#priority bug - superseded)
[1] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-April/214633.htm... [2] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-December/217695.... [3] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-December/217744.... [4] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-April/214987.htm... [5] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html
Christoph, do you want me to update the individual discussions with the most up-to-date status or will you just to plough through them? :)
Thanks, best regards,
^[^ Jaromir Sent from Squeak Inbox Talk On 2021-12-12T15:49:48+01:00, mail at jaromir.net wrote: > Hi All, Christoph, Eliot, > > One more update of new #terminate; I'm sorry for further extending this thread. I've rediscovered how very tricky debugging the BCR is; take this example with nested ensure blocks containg non-local returns: > > [[[] ensure: [^2]] ensure: [^42]] fork > > #terminate needs to avoid returning from the two dangerous contexts while still unwinding as much as possible without a crash :) Also, there's one ensure block inside the debugger opening sequence (inside Debugger class>>openOn:context:... ) which needs to be executed when abandoning the debugger. (I always forget the debugger opening sequence on the top of the stack of the process being debugged is hidden by the debugger) > > I hope the latest #terminate in Kernel-jar.1435 deals with all these issues correctly at last (on top off all other issues and bugs). > > Many thanks for your comments or a review. > > best, > > ~~~ > ^[^ Jaromir > > Sent from Squeak Inbox Talk > > On 2021-12-10T13:45:26+01:00, mail at jaromir.net wrote: > > > Hi Eliot, Christoph, all > > > > Thanks to Eliot's remarks I substantially simplified #terminate's helper method #runUnwind:onBehalfOf: (former #complete:to:) and updated comments. The functionality remains unchanged and all tests pass. > > > > Tested also with the new VM 202112022203. > > > > See Kernel-jar.1434 > > > > Thanks for your comments, > > best, > > > > ~~~ > > ^[^ Jaromir > > > > Sent from Squeak Inbox Talk > > > > On 2021-12-02T17:37:17-08:00, eliot.miranda at gmail.com wrote: > > > > > On Mon, Nov 29, 2021 at 1:46 AM <mail at jaromir.net> wrote: > > > > > > > Hi Eliot, all, > > > > > > > > > This looks good. The one thing I find concerning is running terminate in > > > > a different process. Various idioms could legitimately require an unwind > > > > block to run in the process in which it was created. At the very least > > > > shouldn't the effectiveProcess be set to be that of the process that is > > > > terminating while its unwind block executes? > > > > > > > > > > For example, what if I did something like this: > > > > > doWhileMonitoring: aBlock > > > > > monitoredProcesses add: Processor activeProcess. > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess > > > > ifAbsent: []] > > > > > > > > Would this test prove #terminate works as you'd expect? > > > > ``` > > > > | q | > > > > [ > > > > [q := Processor activeProcess. > > > > Processor activeProcess suspend] ensure: [ > > > > self assert: q = Processor activeProcess] > > > > ] fork. > > > > Processor yield. > > > > q terminate > > > > ``` > > > > > > > > > > Yes, that captures it. > > > > > > > Or with a raised exception: > > > > ``` > > > > | q | > > > > [ > > > > [q := Processor activeProcess. > > > > self error] ensure: [ > > > > self assert: q = Processor activeProcess] > > > > ] fork. > > > > Processor yield. > > > > ``` > > > > (If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, > > > > indeed) > > > > > > > > Please let me know if this is acceptable. > > > > > > > > > > ish. The reason I thought you weren't setting the effective process is that > > > it's buried in complete:to:. I'd rather see it up front in > > > Process>>terminate itself. But I'm happy to defer to you; you've done the > > > work :-) > > > > > > > > > > > > > Thanks again, > > > > > > > > > > > > > > ^[^ Jaromir > > > > -- > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > On 2021-11-28T22:57:25+01:00, mail at jaromir.net wrote: > > > > > > > > > Hi Eliot, > > > > > > > > > > thanks very much for reviewing this. > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > > > in a different process. Various idioms could legitimately require an > > > > unwind block to run in the process in which it was created. > > > > > > > > > > Frankly, I haven't explicitly considered that... In the worst case we > > > > could revert active process termination bit like it was. Terminating an > > > > active process via a different process was just supposed to unify (and > > > > extend) the termination semantics. > > > > > > > > > > > At the very least shouldn't the effectiveProcess be set to be that of > > > > the process that is terminating while its unwind block executes? > > > > > > > > > > That's what I though I did in Process>>#complete: topContext to: > > > > aContext > > > > > > > > > > [...] > > > > > pair := Processor activeProcess > > > > > evaluate: [topContext > > > > runUnwindUntilErrorOrReturnFrom: aContext] > > > > > onBehalfOf: self. > > > > > [...] > > > > > > > > > > where self is the process being terminated. > > > > #runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being > > > > terminated and executes the unwind block with effective process set to the > > > > process being terminated. > > > > > > > > > > Or am I mistaken? > > > > > > > > > > > For example, what if I did something like this: > > > > > > doWhileMonitoring: aBlock > > > > > > monitoredProcesses add: Processor activeProcess. > > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > > > activeProcess ifAbsent: []] > > > > > > > > > > > > That unwind blocks are run in the same process as they were created > > > > seems to me to be a legitimate expectation. Can this be reengineered > > > > maintaining this invariant, or at least setting the effective process to be > > > > that of the process being terminated? > > > > > > > > > > This is a very interesting example - thanks! Running terminate in a > > > > different process allows to relatively easily identify all unwind blocks > > > > half-way through their execution and complete them. In case setting the > > > > effective process in #complete:to: doesn't work as you requested, then I'm > > > > afraid unwinding blocks half-way through their execution would be very hard > > > > to achieve from the process being terminated itself (not impossible though > > > > - I tried but then doubted it was worth the effort). Or theoretically, it > > > > might be possible to maintain two stacks for the process being terminated - > > > > one "original" that needs to be unwound and a second "helper" one running > > > > terminate - but I haven't gotten beyond the idea, it seemed too unchartered > > > > :) > > > > > > > > > > Besides, when an error happens during unwind, a debugger opens and the > > > > identity of the process running terminate may change, so the invariant may > > > > not survive the first error anyway (at least in what I was considering). > > > > > > > > > > > So instead of > > > > > > > > > > > > self isActiveProcess ifTrue: [ > > > > > > [self terminate] fork. > > > > > > ^self suspend]. > > > > > > > > > > > > we would use > > > > > > > > > > > > self isActiveProcess ifTrue: > > > > > > [| arnold | > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > newProcess. > > > > > > arnold resume. > > > > > > ^self suspend]. > > > > > > > > > > > > > > > > No, that wouldn't work because the Terminator identifies itself as the > > > > process being terminated when executing [self terminate] resulting in an > > > > infinite loop. > > > > > > > > > > At the moment I can't see why setting the effective process in > > > > #complete:to: wouldn't suffice but I'll get there :) I'll try to come up > > > > with a test. > > > > > > > > > > I'm intrigued: why did you use '[arnold evaluate:...' and not > > > > '[Processor activeProcess evaluate:...'; I can't figure out the difference > > > > :) > > > > > > > > > > > I might also consider discarding the return of suspend, because it > > > > shouldn't be executed normally, and using, say > > > > > > > > > > > > ``` > > > > > > self isActiveProcess ifTrue: > > > > > > [| arnold | > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > newProcess. > > > > > > arnold resume. > > > > > > self suspend. > > > > > > self error: 'error in terminate; execution resumed after suspend > > > > for termination']. > > > > > > ``` > > > > > > > > > > No, this wouldn't work if Processor activeProcess terminate was inside > > > > an unwind block. Terminate would attempt to proceed after self suspend and > > > > would always raise the error. > > > > > > > > > > Thanks again, > > > > > > > > > > Jaromir > > > > > > > > > > PS: I used this example to test your suggestions: > > > > > > > > > > ``` > > > > > | p | > > > > > p := [ > > > > > [ > > > > > [ ] ensure: [ > > > > > [Processor activeProcess terminate] ensure: [ > > > > > Transcript show: 'x1']. > > > > > Transcript show: 'x2'] > > > > > ] ensure: [ > > > > > Transcript show: 'x3']. > > > > > Transcript show: 'x4' > > > > > ] newProcess. > > > > > p resume. > > > > > "Two yields necessary: terminate active is a two-step procedure" > > > > > Processor yield. Processor yield. > > > > > Transcript show: p isTerminated printString > > > > > > > > > > "prints x1 x2 x3" > > > > > ``` > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > -- > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote: > > > > > > > > > > > Hi Jaromir, Hi Christoph, > > > > > > > > > > > > On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote: > > > > > > > > > > > > > Hi all, Christoph, > > > > > > > > > > > > > > I finally managed to separate #terminate and the 'block cannot return > > > > > > > issue' so that we can discuss them independently :) And thanks to > > > > Eliot's > > > > > > > solution of the infinite recursion in #doesNotUnderstand the > > > > #terminate > > > > > > > code is now cleaner. I haven't included Eliot's modification of > > > > > > > #doesNotUnderstand in my changeset but paste it below instead > > > > (because I'm > > > > > > > not the author; I hope it'll be merged though): > > > > > > > > > > > > > > The #terminate code presented in Kernel-jar.1426 is the final > > > > version of > > > > > > > my effort to get rid of all the bugs reported in this thread (and > > > > make > > > > > > > #terminate semantics more consistent). It passes all standard tests > > > > and the > > > > > > > tests complementing this changeset: > > > > > > > > > > > > > > KernelTests-jar.406 (Terminator test) > > > > > > > KernelTests-jar.407 (McClure test) > > > > > > > Tests-jar.466 (unwind tests) > > > > > > > ToolsTests-jar.105 (debugger tests) > > > > > > > > > > > > > > In addition, it's fully compatible with Christoph's solution of the > > > > 'block > > > > > > > cannot return issue' presented in Kernel-ct.1405 ( > > > > > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html > > > > ). > > > > > > > > > > > > > > > > > > > > > Summary and discussion about the bugs and changes in #terminate: > > > > > > > > > > > http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal-td5128285.html > > > > > > > > > > > > > > I think it's ready for merging... Please review. > > > > > > > > > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > > > in a > > > > > > different process. Various idioms could legitimately require an unwind > > > > > > block to run in the process in which it was created. At the very least > > > > > > shouldn't the effectiveProcess be set to be that of the process that is > > > > > > terminating while its unwind block executes? > > > > > > > > > > > > For example, what if I did something like this: > > > > > > doWhileMonitoring: aBlock > > > > > > monitoredProcesses add: Processor activeProcess. > > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > > > activeProcess > > > > > > ifAbsent: []] > > > > > > > > > > > > That unwind blocks are run in the same process as they were created > > > > seems > > > > > > to me to be a legitimate expectation. Can this be reengineered > > > > maintaining > > > > > > this invariant, or at least setting the effective process to be that > > > > of the > > > > > > process being terminated? > > > > > > > > > > > > So instead of > > > > > > > > > > > > self isActiveProcess ifTrue: [ > > > > > > [self terminate] fork. > > > > > > ^self suspend]. > > > > > > > > > > > > we would use > > > > > > > > > > > > self isActiveProcess ifTrue: > > > > > > [| arnold | > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > > newProcess. > > > > > > arnold resume. > > > > > > ^self suspend]. > > > > > > > > > > > > I might also consider discarding the return of suspend, because it > > > > > > shouldn't be executed normally, and using, say > > > > > > > > > > > > self isActiveProcess ifTrue: > > > > > > [| arnold | > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > > newProcess. > > > > > > arnold resume. > > > > > > self suspend. > > > > > > self error: 'error in terminate; execution resumed after > > > > suspend > > > > > > for termination']. > > > > > > > > > > > > > > > > > > > -- > > > > > > > Eliot's fix of infinite recursion in doesNotUnderstand: > > > > > > > > > > > > > > > > > > > > > doesNotUnderstand: aMessage > > > > > > > "Handle the fact that there was an attempt to send the given > > > > > > > message to the receiver but the receiver does not > > > > understand > > > > > > > this message (typically sent from the machine when a > > > > message > > > > > > > is sent to the receiver and no method is defined for that > > > > > > > selector)." > > > > > > > > > > > > > > "Testing: (3 activeProcess)" > > > > > > > > > > > > > > | exception resumeValue | > > > > > > > (exception := MessageNotUnderstood new) > > > > > > > message: aMessage; > > > > > > > receiver: self. > > > > > > > resumeValue := exception signal. > > > > > > > ^exception reachedDefaultHandler > > > > > > > ifFalse: [resumeValue] > > > > > > > "----------> this wrapper detecting recursion is added: > > > > -------------->" > > > > > > > > > > > > > > ifTrue: [ > > > > > > > [aMessage sentTo: self] > > > > > > > on: MessageNotUnderstood > > > > > > > do: [:ex| | args | > > > > > > > args := ex message arguments. > > > > > > > (ex receiver == self > > > > > > > and: [ex message selector == > > > > > > > aMessage selector > > > > > > > and: [(1 to: aMessage > > > > numArgs) > > > > > > > allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) > > > > ifFalse: > > > > > > > [ex pass]. > > > > > > > self error: 'infinite recursion > > > > in > > > > > > > doesNotUnderstand:']] > > > > > > > > > > > > > > (see discussion > > > > > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.html > > > > > > > ) > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > > -- > > > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote: > > > > > > > > > > > > > > > Hi Christoph, > > > > > > > > > > > > > > > > Once more, apologies for taking so long to respond. > > > > > > > > > > > > > > > > > > [...] there really are two distinct unwind semantics : one > > > > "light" > > > > > > > for regular returns and one "heavy" for termination. Both are very > > > > similar > > > > > > > yet each require a slightly different behavior - that's why the > > > > duality > > > > > > > #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or > > > > #complete: > > > > > > > / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > > > extra > > > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > > > code > > > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > > > Yes, I understand your concern and agree two ifs would allow to > > > > pack the > > > > > > > two twin beasts into one can (I refer to "rueorf" & "ruueorf"). > > > > Also, we > > > > > > > could consider Process >> complete:to: as a generalized version of > > > > Process > > > > > > > >> complete and rewrite it accordingly. This is doable, maybe even > > > > > > > desirable but for the time being I'd very much prefer to keep them > > > > separate > > > > > > > to facilitate troubleshooting of potential problems (or localizing > > > > them). > > > > > > > Each of the two similar methods belong to a different execution path > > > > after > > > > > > > all. And besides, #runUntilErrorOrReturnFrom is already crazy enough > > > > and I > > > > > > > don't feel like making it even more complex now :) > > > > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > > > wondering whether it wouldn't have the same unwind problem with > > > > non-local > > > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > > > described in > > > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > > > I put #unwindTo: aside for the moment; it contains the same flaw > > > > and > > > > > > > deserves its own thread. > > > > > > > > > > > > > > > > > Unfortunately I've lost track of these infinite loops [...] > > > > > > > > > > > > > > > > Yeah, tell me about it :) > > > > > > > > > > > > > > > > > [...]- could you maybe point me to some concrete examples that > > > > lead to > > > > > > > an infinite recursion without this special check? :-) > > > > > > > > > > > > > > > > Try: > > > > > > > > > > > > > > > > ``` > > > > > > > > [self error: 'error'] ensure: [self gotcha. Transcript show: 'all > > > > > > > cleaned up! '] > > > > > > > > ``` > > > > > > > > > > > > > > > > 'gotcha' represents a method unknown to the system. The code will > > > > print > > > > > > > 'all cleaned up! ' after abandoning the error and the debugger - > > > > meaning > > > > > > > the system will survive this code and execute all cleanups (aka > > > > unwinds). > > > > > > > In case you use your #terminateAggressively as the default action for > > > > > > > abandoning the Debugger, you won't have any problem of course > > > > because you > > > > > > > skip completing the halfway through unwind. What I'm saying is: even > > > > if you > > > > > > > chose the classic heavy #terminate to exit the Debugger, the code > > > > will > > > > > > > execute remaining unwinds as one might expect (or hope for). > > > > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > > > Are you > > > > > > > maybe missing something like "cxt terminate" in the "No error was > > > > raised" > > > > > > > case? Just wondering. > > > > > > > > > > > > > > > > I guess I know what you mean... 'ctxt' would get garbage collected > > > > soon > > > > > > > anyway so I left it as is to keep the code as simple as possible. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Christoph, I'll also address your comments regarding the > > > > > > > BlockCannotReturn behavior here: > > > > > > > > > > > > > > > > > Isn't that ProceedBlockCannotReturn tautologous? I think that by > > > > > > > actively proceeding from a BlockCannotReturn error users already > > > > accept > > > > > > > that they are going to resume execution in another way. > > > > > > > > > > > > > > > > Well, the very proceeding from a BlockCannotReturn error sort of > > > > > > > violates common sense but during our lengthy discussion you > > > > convinced me it > > > > > > > makes a very good sense when troubleshooting :) The idea is by no > > > > means > > > > > > > trivial - unlike hitting Proceed :) So an extra warning can't hurt... > > > > > > > > > > > > > > > > But more importantly, I need something to know the user let the > > > > process > > > > > > > continue after reaching the BlockCannotReturn error - thus the new > > > > > > > ProceedBlockCannotReturn exception which allows Process >> > > > > #complete:to: to > > > > > > > deal with the new course of events. > > > > > > > > > > > > > > > > > Apart from that, the message text of your new warning is not > > > > correct > > > > > > > if self pc <= self endPC. :-) > > > > > > > > > > > > > > > > Yes, and I'd like to make the warning message more verbose so even > > > > if > > > > > > > someone hit Proceed without much thinking they could get an idea > > > > what's > > > > > > > about to happen :) Should the warning interfere with some potential > > > > > > > automation efforts we could come up with some alternative way. > > > > > > > > > > > > > > > > I'll copy my reply regarding BCR to the "The Inbox: > > > > Kernel-ct.1405.mcz" > > > > > > > thread and we can continue discussing this particular issue there. > > > > It's > > > > > > > really a separate issue and I included your patch here along with > > > > the main > > > > > > > termination code because it perfectly complements it and prevents the > > > > > > > disastrous crashes caused by Proceeding the BCR error. > > > > > > > > > > > > > > > > > [...] the computation terminated is also wrong, IMO, you should > > > > get a > > > > > > > BlockCannotReturn here. > > > > > > > > > > > > > > > > Yes, I agree, there's definitely more to it which deserves to be > > > > > > > discussed thoroughly and separately in the "The Inbox: > > > > Kernel-ct.1405.mcz" > > > > > > > thread. > > > > > > > > > > > > > > > > If you agree I'd really appreciate if your fix could be accepted > > > > here in > > > > > > > including the ProceedBlockCannotReturn exception I need to make it > > > > work > > > > > > > together with the code I'm presenting here. If we come up with a > > > > better > > > > > > > idea in the other discussion we can make amends here as well; I take > > > > it as > > > > > > > a patch, not a definitive solution :) > > > > > > > > > > > > > > > > Thanks again so much for all your suggestions and examples! It's > > > > always > > > > > > > a pleasure :) Plus - the Inbox Talk is priceless... I use it > > > > exclusively > > > > > > > now and look forward to any improvements you might come up in the > > > > future ;) > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > > > -- > > > > > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > > > On 2021-08-22T16:49:10+02:00, christoph.thiede at > > > > > > > student.hpi.uni-potsdam.de wrote: > > > > > > > > > > > > > > > > > Hi Jaromir, > > > > > > > > > > > > > > > > > > > Yes, I was wondering why I couldn't get rid of the duplication > > > > and > > > > > > > now I think it's because there really are two distinct unwind > > > > semantics : > > > > > > > one "light" for regular returns and one "heavy" for termination. > > > > Both are > > > > > > > very similar yet each require a slightly different behavior - that's > > > > why > > > > > > > the duality #runUntilErrorOrReturnFrom / > > > > #runUnwindUntilErrorOrReturnFrom > > > > > > > or #complete: / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > > > extra > > > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > > > code > > > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > > > wondering whether it wouldn't have the same unwind problem with > > > > non-local > > > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > > > described in > > > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > > > > > > But in general - yes, any method/exception purposefully (or > > > > not) > > > > > > > written to create a loop will break this patch (I admit it is just a > > > > patch > > > > > > > really). I extracted it to #complete:to: to make #terminate clean; > > > > this is > > > > > > > a WIP; I wish there was a holistic solution to this - maybe checking > > > > for > > > > > > > exception recursion by default? :) > > > > > > > > > > > > > > > > > > Sounds better already, if feasible! But how would you detect > > > > this? > > > > > > > Unfortunately I've lost track of these infinite loops - could you > > > > maybe > > > > > > > point me to some concrete examples that lead to an infinite recursion > > > > > > > without this special check? :-) > > > > > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > > > Are you > > > > > > > maybe missing something like "cxt terminate" in the "No error was > > > > raised" > > > > > > > case? Just wondering. > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Christoph > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > > > --- > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > > > > > On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote: > > > > > > > > > > > > > > > > > > > Jaromir Matas wrote > > > > > > > > > > > Hi All, > > > > > > > > > > > I've sent an updated version of #teminate integrating > > > > Christoph's > > > > > > > solution > > > > > > > > > > > of BlockCannotReturn recursion problem (in [1]), along with a > > > > > > > battery of > > > > > > > > > > > tests exploring termination of nested ensure and cascading > > > > errors > > > > > > > behavior > > > > > > > > > > > (Debugger tests are for info and a final version can wait > > > > until > > > > > > > releasing > > > > > > > > > > > Christoph's proposal in [2]). > > > > > > > > > > > > > > > > > > > > > > It's pretty much final, I hope... > > > > > > > > > > > > > > > > > > > > > > Any complaints about #terminate - please shout ;) > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html > > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > > > Here's the link: > > > > > > > > > > > > > > http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- > > > > > > > > > > ^[^ Jaromir > > > > > > > > > > -- > > > > > > > > > > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
--==CelesteAttachment41815== Content-type: text/plain;charset=UTF-8
Hi all, Eliot, Marcel, Christoph,
I'm enclosing the next iteration of #terminate; this time the unwind blocks are always executed directly in the same process as they were created, instead of indirectly from another process. #terminate creates a parallel stack for the process being terminated to unwind itself from, regardless of whether it's active or suspended.
This way we maintain the invariant that unwind blocks are run in the same process as they were created.
In addition, now the problem of multiple termination or resumption of the process being terminated becomes a non-issue (almost) because active process's suspendedContext is nil.
The unwind pattern used for unwinding is a generalized version of Context>>unwindTo: As a side effect, it can replace Context>>unwindTo: used currently by the Generator, and fix a bug in the current #unwindTo: at the same time. All tests are green indeed.
If you're interested I could update all the comments in the tests I already submitted to the Inbox.
Please check it out and let me know if you have any concerns.
#terminate now reads: (this is for the revised suspend semantics in the latest 2022 VM)
| top | self isActiveProcess ifTrue: [ top := thisContext. ^[top unwindTo: nil. self suspend] asContext jump].
[] ensure: [ suspendedContext isContext ifFalse: [^self]. self suspend. top := suspendedContext. suspendedContext := [top releaseCriticalSection; unwindTo: nil. self suspend] asContext. self priority: Processor activePriority + 1; resume]
(full version in the enclosed changeset)
Thanks for your comments,
best, ~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk
On 2022-01-05T15:30:25+01:00, mail@jaromir.net wrote:
Hi Eliot, all,
I hear your concerns about running terminate in a different process.
So for me, KISS. Just support something that guards against multiple termination, and in the debugger, reveals clearly where a process is in the course of running unwinds.
And I really *don'* like the idea of running terminate in a separate process. This has bad performance implications. Most terminations can be done in the process itself; that should be the expectation.
Agreed, I didn't like it either.
That unwind blocks are run in the same process as they were created seems to me to be a legitimate expectation. Can this be reengineered maintaining this invariant?
To my surprise the solution appears to be quite straightforward: for a terminating process, create a parallel stack and run the process's own unwinds there; all tests pass and I'm very anxious to hear your opinion about possible drawbacks of one process executing on two stacks.
#terminate then reads:
terminate | top | "If terminating the active process, create a parallel stack and run unwinds from there." self isActiveProcess ifTrue: [ top := thisContext. ^[self unwind: top; suspend] asContext jump]. [] ensure: [ suspendedContext ifNil: [^self error: 'Process already terminated or terminating']. Smalltalk processSuspensionUnblocks ifFalse: [ "this part is for revised resume semantics introduced in 2022 VMs" self suspend. top := suspendedContext. suspendedContext := nil. top releaseCriticalSection. self unwind: top] ifTrue: [ "this part is for backward compatibilty with pre-2022 VMs" | oldList | oldList := self suspend. self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]). top := suspendedContext. suspendedContext := nil. self unwind: top]]
The whole version including comments and the helper methods #unwind: and #unwind:to: is in Kernel-jar.1443.
I've also included a simplified version of #releaseCriticalSection (which is now under Context).
This version of #terminate is backward compatible with older VMs (without the revised suspend).
A battery of updated tests is in KernelTests-jar.421.
Thanks for any comments.
best,
^[^ Jaromir Sent from Squeak Inbox Talk On 2021-12-22T18:22:50+01:00, mail at jaromir.net wrote: > Hi Marcel, Christoph and all, > > Let me summarize the current state of the #terminate fix (Kernel-jar.1437): > > There's been a development in the issue recently; two more bugs have surfaced and even if they are solvable independently of the latest #terminate fix I chose to include them in the #teminate changeset to, hopefully, facilitate the integration. > > So the changeset Kernel-jar.1437 addresses all of: > > (1) the original termination issues from [1] > (2) multiple termination (/resumption) issue from [2] > (3) #releaseCriticalSection issue from [3] > > In case you decide to integrate fixes for (2) and (3) independently I can adjust #terminate accordingly following those fixes. > > In addition there are two more related issues (but again, independent of the latest #terminate): > > (4) stepOver bug solution discussed in [4] and solved in Kernel-jar.1415 > (5) proceedable #cannotReturn extension by Christoph discussed in [5] and presented in Kernel-ct.1405 > > I haven't included these two in #terminate because the solutions don't interact with #terminate and in case of (5) Christoph's the author. > > Here's *my opinion* about the status of each: > (1) ready > (2) proposed solution works but may require some (cosmetic) polishing > (3) ready > (4) ready > (5) I like it > > The following tests in the Inbox are still relevant: > > KernelTests-jar.420 (releaseCriticalSection bug) > KernelTests-jar.417 (multiple termination / resumption) > KernelTests-jar.416 (ensure as top) > KernelTests-jar.415 (unwind tests) > KernelTests-jar.406 (nested termination) > ToolsTests-jar.109 (debugger termination) > > To cleanup the Inbox, the following changesets can be moved to Treated: > > Tests-jar.466 (unwind tests - superseded) > Kernel-jar.1435 (#terminate - superseded) > Kernel-jar.1426 (#terminate - superseded) > Kernel-jar.1413 (stepOver bug - superseded) > Kernel-jar.1421 (stepOver bug - superseded) > KernelTests-jar.419 (releaseCriticalSection bug - superseded) > KernelTests-jar.407 (McClure test - withdrawing) > ToolsTests-jar.105 (debugger tests - superseded) > KernelTests-jar.393 (#priority bug - superseded) > > [1] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-April/214633.html > [2] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-December/217695.html > [3] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-December/217744.html > [4] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-April/214987.html > [5] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html > > Christoph, do you want me to update the individual discussions with the most up-to-date status or will you just to plough through them? :) > > Thanks, > best regards, > > > ~~~ > ^[^ Jaromir > > Sent from Squeak Inbox Talk > > On 2021-12-12T15:49:48+01:00, mail at jaromir.net wrote: > > > Hi All, Christoph, Eliot, > > > > One more update of new #terminate; I'm sorry for further extending this thread. I've rediscovered how very tricky debugging the BCR is; take this example with nested ensure blocks containg non-local returns: > > > > [[[] ensure: [^2]] ensure: [^42]] fork > > > > #terminate needs to avoid returning from the two dangerous contexts while still unwinding as much as possible without a crash :) Also, there's one ensure block inside the debugger opening sequence (inside Debugger class>>openOn:context:... ) which needs to be executed when abandoning the debugger. (I always forget the debugger opening sequence on the top of the stack of the process being debugged is hidden by the debugger) > > > > I hope the latest #terminate in Kernel-jar.1435 deals with all these issues correctly at last (on top off all other issues and bugs). > > > > Many thanks for your comments or a review. > > > > best, > > > > ~~~ > > ^[^ Jaromir > > > > Sent from Squeak Inbox Talk > > > > On 2021-12-10T13:45:26+01:00, mail at jaromir.net wrote: > > > > > Hi Eliot, Christoph, all > > > > > > Thanks to Eliot's remarks I substantially simplified #terminate's helper method #runUnwind:onBehalfOf: (former #complete:to:) and updated comments. The functionality remains unchanged and all tests pass. > > > > > > Tested also with the new VM 202112022203. > > > > > > See Kernel-jar.1434 > > > > > > Thanks for your comments, > > > best, > > > > > > ~~~ > > > ^[^ Jaromir > > > > > > Sent from Squeak Inbox Talk > > > > > > On 2021-12-02T17:37:17-08:00, eliot.miranda at gmail.com wrote: > > > > > > > On Mon, Nov 29, 2021 at 1:46 AM <mail at jaromir.net> wrote: > > > > > > > > > Hi Eliot, all, > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate in > > > > > a different process. Various idioms could legitimately require an unwind > > > > > block to run in the process in which it was created. At the very least > > > > > shouldn't the effectiveProcess be set to be that of the process that is > > > > > terminating while its unwind block executes? > > > > > > > > > > > > For example, what if I did something like this: > > > > > > doWhileMonitoring: aBlock > > > > > > monitoredProcesses add: Processor activeProcess. > > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor activeProcess > > > > > ifAbsent: []] > > > > > > > > > > Would this test prove #terminate works as you'd expect? > > > > > ``` > > > > > | q | > > > > > [ > > > > > [q := Processor activeProcess. > > > > > Processor activeProcess suspend] ensure: [ > > > > > self assert: q = Processor activeProcess] > > > > > ] fork. > > > > > Processor yield. > > > > > q terminate > > > > > ``` > > > > > > > > > > > > > Yes, that captures it. > > > > > > > > > Or with a raised exception: > > > > > ``` > > > > > | q | > > > > > [ > > > > > [q := Processor activeProcess. > > > > > self error] ensure: [ > > > > > self assert: q = Processor activeProcess] > > > > > ] fork. > > > > > Processor yield. > > > > > ``` > > > > > (If I remove #evaluate:onBehalfOf: from #complete:to: both tests fail, > > > > > indeed) > > > > > > > > > > Please let me know if this is acceptable. > > > > > > > > > > > > > ish. The reason I thought you weren't setting the effective process is that > > > > it's buried in complete:to:. I'd rather see it up front in > > > > Process>>terminate itself. But I'm happy to defer to you; you've done the > > > > work :-) > > > > > > > > > > > > > > > > > Thanks again, > > > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > -- > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > On 2021-11-28T22:57:25+01:00, mail at jaromir.net wrote: > > > > > > > > > > > Hi Eliot, > > > > > > > > > > > > thanks very much for reviewing this. > > > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > > > > in a different process. Various idioms could legitimately require an > > > > > unwind block to run in the process in which it was created. > > > > > > > > > > > > Frankly, I haven't explicitly considered that... In the worst case we > > > > > could revert active process termination bit like it was. Terminating an > > > > > active process via a different process was just supposed to unify (and > > > > > extend) the termination semantics. > > > > > > > > > > > > > At the very least shouldn't the effectiveProcess be set to be that of > > > > > the process that is terminating while its unwind block executes? > > > > > > > > > > > > That's what I though I did in Process>>#complete: topContext to: > > > > > aContext > > > > > > > > > > > > [...] > > > > > > pair := Processor activeProcess > > > > > > evaluate: [topContext > > > > > runUnwindUntilErrorOrReturnFrom: aContext] > > > > > > onBehalfOf: self. > > > > > > [...] > > > > > > > > > > > > where self is the process being terminated. > > > > > #runUnwindUntilErrorOrReturnFrom: jumps to the stack of the process being > > > > > terminated and executes the unwind block with effective process set to the > > > > > process being terminated. > > > > > > > > > > > > Or am I mistaken? > > > > > > > > > > > > > For example, what if I did something like this: > > > > > > > doWhileMonitoring: aBlock > > > > > > > monitoredProcesses add: Processor activeProcess. > > > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > > > > activeProcess ifAbsent: []] > > > > > > > > > > > > > > That unwind blocks are run in the same process as they were created > > > > > seems to me to be a legitimate expectation. Can this be reengineered > > > > > maintaining this invariant, or at least setting the effective process to be > > > > > that of the process being terminated? > > > > > > > > > > > > This is a very interesting example - thanks! Running terminate in a > > > > > different process allows to relatively easily identify all unwind blocks > > > > > half-way through their execution and complete them. In case setting the > > > > > effective process in #complete:to: doesn't work as you requested, then I'm > > > > > afraid unwinding blocks half-way through their execution would be very hard > > > > > to achieve from the process being terminated itself (not impossible though > > > > > - I tried but then doubted it was worth the effort). Or theoretically, it > > > > > might be possible to maintain two stacks for the process being terminated - > > > > > one "original" that needs to be unwound and a second "helper" one running > > > > > terminate - but I haven't gotten beyond the idea, it seemed too unchartered > > > > > :) > > > > > > > > > > > > Besides, when an error happens during unwind, a debugger opens and the > > > > > identity of the process running terminate may change, so the invariant may > > > > > not survive the first error anyway (at least in what I was considering). > > > > > > > > > > > > > So instead of > > > > > > > > > > > > > > self isActiveProcess ifTrue: [ > > > > > > > [self terminate] fork. > > > > > > > ^self suspend]. > > > > > > > > > > > > > > we would use > > > > > > > > > > > > > > self isActiveProcess ifTrue: > > > > > > > [| arnold | > > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > newProcess. > > > > > > > arnold resume. > > > > > > > ^self suspend]. > > > > > > > > > > > > > > > > > > > No, that wouldn't work because the Terminator identifies itself as the > > > > > process being terminated when executing [self terminate] resulting in an > > > > > infinite loop. > > > > > > > > > > > > At the moment I can't see why setting the effective process in > > > > > #complete:to: wouldn't suffice but I'll get there :) I'll try to come up > > > > > with a test. > > > > > > > > > > > > I'm intrigued: why did you use '[arnold evaluate:...' and not > > > > > '[Processor activeProcess evaluate:...'; I can't figure out the difference > > > > > :) > > > > > > > > > > > > > I might also consider discarding the return of suspend, because it > > > > > shouldn't be executed normally, and using, say > > > > > > > > > > > > > > ``` > > > > > > > self isActiveProcess ifTrue: > > > > > > > [| arnold | > > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > newProcess. > > > > > > > arnold resume. > > > > > > > self suspend. > > > > > > > self error: 'error in terminate; execution resumed after suspend > > > > > for termination']. > > > > > > > ``` > > > > > > > > > > > > No, this wouldn't work if Processor activeProcess terminate was inside > > > > > an unwind block. Terminate would attempt to proceed after self suspend and > > > > > would always raise the error. > > > > > > > > > > > > Thanks again, > > > > > > > > > > > > Jaromir > > > > > > > > > > > > PS: I used this example to test your suggestions: > > > > > > > > > > > > ``` > > > > > > | p | > > > > > > p := [ > > > > > > [ > > > > > > [ ] ensure: [ > > > > > > [Processor activeProcess terminate] ensure: [ > > > > > > Transcript show: 'x1']. > > > > > > Transcript show: 'x2'] > > > > > > ] ensure: [ > > > > > > Transcript show: 'x3']. > > > > > > Transcript show: 'x4' > > > > > > ] newProcess. > > > > > > p resume. > > > > > > "Two yields necessary: terminate active is a two-step procedure" > > > > > > Processor yield. Processor yield. > > > > > > Transcript show: p isTerminated printString > > > > > > > > > > > > "prints x1 x2 x3" > > > > > > ``` > > > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > -- > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > On 2021-11-28T10:52:41-08:00, eliot.miranda at gmail.com wrote: > > > > > > > > > > > > > Hi Jaromir, Hi Christoph, > > > > > > > > > > > > > > On Sat, Nov 27, 2021 at 12:12 PM <mail at jaromir.net> wrote: > > > > > > > > > > > > > > > Hi all, Christoph, > > > > > > > > > > > > > > > > I finally managed to separate #terminate and the 'block cannot return > > > > > > > > issue' so that we can discuss them independently :) And thanks to > > > > > Eliot's > > > > > > > > solution of the infinite recursion in #doesNotUnderstand the > > > > > #terminate > > > > > > > > code is now cleaner. I haven't included Eliot's modification of > > > > > > > > #doesNotUnderstand in my changeset but paste it below instead > > > > > (because I'm > > > > > > > > not the author; I hope it'll be merged though): > > > > > > > > > > > > > > > > The #terminate code presented in Kernel-jar.1426 is the final > > > > > version of > > > > > > > > my effort to get rid of all the bugs reported in this thread (and > > > > > make > > > > > > > > #terminate semantics more consistent). It passes all standard tests > > > > > and the > > > > > > > > tests complementing this changeset: > > > > > > > > > > > > > > > > KernelTests-jar.406 (Terminator test) > > > > > > > > KernelTests-jar.407 (McClure test) > > > > > > > > Tests-jar.466 (unwind tests) > > > > > > > > ToolsTests-jar.105 (debugger tests) > > > > > > > > > > > > > > > > In addition, it's fully compatible with Christoph's solution of the > > > > > 'block > > > > > > > > cannot return issue' presented in Kernel-ct.1405 ( > > > > > > > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-May/215526.html > > > > > ). > > > > > > > > > > > > > > > > > > > > > > > > Summary and discussion about the bugs and changes in #terminate: > > > > > > > > > > > > > http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal-td5128285.html > > > > > > > > > > > > > > > > I think it's ready for merging... Please review. > > > > > > > > > > > > > > > > > > > > > > This looks good. The one thing I find concerning is running terminate > > > > > in a > > > > > > > different process. Various idioms could legitimately require an unwind > > > > > > > block to run in the process in which it was created. At the very least > > > > > > > shouldn't the effectiveProcess be set to be that of the process that is > > > > > > > terminating while its unwind block executes? > > > > > > > > > > > > > > For example, what if I did something like this: > > > > > > > doWhileMonitoring: aBlock > > > > > > > monitoredProcesses add: Processor activeProcess. > > > > > > > ^aBlock ensure: [monitoredProcesses remove:: Processor > > > > > activeProcess > > > > > > > ifAbsent: []] > > > > > > > > > > > > > > That unwind blocks are run in the same process as they were created > > > > > seems > > > > > > > to me to be a legitimate expectation. Can this be reengineered > > > > > maintaining > > > > > > > this invariant, or at least setting the effective process to be that > > > > > of the > > > > > > > process being terminated? > > > > > > > > > > > > > > So instead of > > > > > > > > > > > > > > self isActiveProcess ifTrue: [ > > > > > > > [self terminate] fork. > > > > > > > ^self suspend]. > > > > > > > > > > > > > > we would use > > > > > > > > > > > > > > self isActiveProcess ifTrue: > > > > > > > [| arnold | > > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > > > newProcess. > > > > > > > arnold resume. > > > > > > > ^self suspend]. > > > > > > > > > > > > > > I might also consider discarding the return of suspend, because it > > > > > > > shouldn't be executed normally, and using, say > > > > > > > > > > > > > > self isActiveProcess ifTrue: > > > > > > > [| arnold | > > > > > > > arnold := [arnold evaluate: [self terminate] onBehalfOf: self] > > > > > > > newProcess. > > > > > > > arnold resume. > > > > > > > self suspend. > > > > > > > self error: 'error in terminate; execution resumed after > > > > > suspend > > > > > > > for termination']. > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Eliot's fix of infinite recursion in doesNotUnderstand: > > > > > > > > > > > > > > > > > > > > > > > > doesNotUnderstand: aMessage > > > > > > > > "Handle the fact that there was an attempt to send the given > > > > > > > > message to the receiver but the receiver does not > > > > > understand > > > > > > > > this message (typically sent from the machine when a > > > > > message > > > > > > > > is sent to the receiver and no method is defined for that > > > > > > > > selector)." > > > > > > > > > > > > > > > > "Testing: (3 activeProcess)" > > > > > > > > > > > > > > > > | exception resumeValue | > > > > > > > > (exception := MessageNotUnderstood new) > > > > > > > > message: aMessage; > > > > > > > > receiver: self. > > > > > > > > resumeValue := exception signal. > > > > > > > > ^exception reachedDefaultHandler > > > > > > > > ifFalse: [resumeValue] > > > > > > > > "----------> this wrapper detecting recursion is added: > > > > > -------------->" > > > > > > > > > > > > > > > > ifTrue: [ > > > > > > > > [aMessage sentTo: self] > > > > > > > > on: MessageNotUnderstood > > > > > > > > do: [:ex| | args | > > > > > > > > args := ex message arguments. > > > > > > > > (ex receiver == self > > > > > > > > and: [ex message selector == > > > > > > > > aMessage selector > > > > > > > > and: [(1 to: aMessage > > > > > numArgs) > > > > > > > > allSatisfy: [:i| (args at: i) == (aMessage argumentAt: i)]]]) > > > > > ifFalse: > > > > > > > > [ex pass]. > > > > > > > > self error: 'infinite recursion > > > > > in > > > > > > > > doesNotUnderstand:']] > > > > > > > > > > > > > > > > (see discussion > > > > > > > > > > > > > http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217031.html > > > > > > > > ) > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > > > -- > > > > > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > > > On 2021-11-17T18:49:44+01:00, mail at jaromir.net wrote: > > > > > > > > > > > > > > > > > Hi Christoph, > > > > > > > > > > > > > > > > > > Once more, apologies for taking so long to respond. > > > > > > > > > > > > > > > > > > > > [...] there really are two distinct unwind semantics : one > > > > > "light" > > > > > > > > for regular returns and one "heavy" for termination. Both are very > > > > > similar > > > > > > > > yet each require a slightly different behavior - that's why the > > > > > duality > > > > > > > > #runUntilErrorOrReturnFrom / #runUnwindUntilErrorOrReturnFrom or > > > > > #complete: > > > > > > > > / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > > > > extra > > > > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > > > > code > > > > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > > > > > Yes, I understand your concern and agree two ifs would allow to > > > > > pack the > > > > > > > > two twin beasts into one can (I refer to "rueorf" & "ruueorf"). > > > > > Also, we > > > > > > > > could consider Process >> complete:to: as a generalized version of > > > > > Process > > > > > > > > >> complete and rewrite it accordingly. This is doable, maybe even > > > > > > > > desirable but for the time being I'd very much prefer to keep them > > > > > separate > > > > > > > > to facilitate troubleshooting of potential problems (or localizing > > > > > them). > > > > > > > > Each of the two similar methods belong to a different execution path > > > > > after > > > > > > > > all. And besides, #runUntilErrorOrReturnFrom is already crazy enough > > > > > and I > > > > > > > > don't feel like making it even more complex now :) > > > > > > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > > > > wondering whether it wouldn't have the same unwind problem with > > > > > non-local > > > > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > > > > described in > > > > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > > > > > I put #unwindTo: aside for the moment; it contains the same flaw > > > > > and > > > > > > > > deserves its own thread. > > > > > > > > > > > > > > > > > > > Unfortunately I've lost track of these infinite loops [...] > > > > > > > > > > > > > > > > > > Yeah, tell me about it :) > > > > > > > > > > > > > > > > > > > [...]- could you maybe point me to some concrete examples that > > > > > lead to > > > > > > > > an infinite recursion without this special check? :-) > > > > > > > > > > > > > > > > > > Try: > > > > > > > > > > > > > > > > > > ``` > > > > > > > > > [self error: 'error'] ensure: [self gotcha. Transcript show: 'all > > > > > > > > cleaned up! '] > > > > > > > > > ``` > > > > > > > > > > > > > > > > > > 'gotcha' represents a method unknown to the system. The code will > > > > > print > > > > > > > > 'all cleaned up! ' after abandoning the error and the debugger - > > > > > meaning > > > > > > > > the system will survive this code and execute all cleanups (aka > > > > > unwinds). > > > > > > > > In case you use your #terminateAggressively as the default action for > > > > > > > > abandoning the Debugger, you won't have any problem of course > > > > > because you > > > > > > > > skip completing the halfway through unwind. What I'm saying is: even > > > > > if you > > > > > > > > chose the classic heavy #terminate to exit the Debugger, the code > > > > > will > > > > > > > > execute remaining unwinds as one might expect (or hope for). > > > > > > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > > > > Are you > > > > > > > > maybe missing something like "cxt terminate" in the "No error was > > > > > raised" > > > > > > > > case? Just wondering. > > > > > > > > > > > > > > > > > > I guess I know what you mean... 'ctxt' would get garbage collected > > > > > soon > > > > > > > > anyway so I left it as is to keep the code as simple as possible. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Christoph, I'll also address your comments regarding the > > > > > > > > BlockCannotReturn behavior here: > > > > > > > > > > > > > > > > > > > Isn't that ProceedBlockCannotReturn tautologous? I think that by > > > > > > > > actively proceeding from a BlockCannotReturn error users already > > > > > accept > > > > > > > > that they are going to resume execution in another way. > > > > > > > > > > > > > > > > > > Well, the very proceeding from a BlockCannotReturn error sort of > > > > > > > > violates common sense but during our lengthy discussion you > > > > > convinced me it > > > > > > > > makes a very good sense when troubleshooting :) The idea is by no > > > > > means > > > > > > > > trivial - unlike hitting Proceed :) So an extra warning can't hurt... > > > > > > > > > > > > > > > > > > But more importantly, I need something to know the user let the > > > > > process > > > > > > > > continue after reaching the BlockCannotReturn error - thus the new > > > > > > > > ProceedBlockCannotReturn exception which allows Process >> > > > > > #complete:to: to > > > > > > > > deal with the new course of events. > > > > > > > > > > > > > > > > > > > Apart from that, the message text of your new warning is not > > > > > correct > > > > > > > > if self pc <= self endPC. :-) > > > > > > > > > > > > > > > > > > Yes, and I'd like to make the warning message more verbose so even > > > > > if > > > > > > > > someone hit Proceed without much thinking they could get an idea > > > > > what's > > > > > > > > about to happen :) Should the warning interfere with some potential > > > > > > > > automation efforts we could come up with some alternative way. > > > > > > > > > > > > > > > > > > I'll copy my reply regarding BCR to the "The Inbox: > > > > > Kernel-ct.1405.mcz" > > > > > > > > thread and we can continue discussing this particular issue there. > > > > > It's > > > > > > > > really a separate issue and I included your patch here along with > > > > > the main > > > > > > > > termination code because it perfectly complements it and prevents the > > > > > > > > disastrous crashes caused by Proceeding the BCR error. > > > > > > > > > > > > > > > > > > > [...] the computation terminated is also wrong, IMO, you should > > > > > get a > > > > > > > > BlockCannotReturn here. > > > > > > > > > > > > > > > > > > Yes, I agree, there's definitely more to it which deserves to be > > > > > > > > discussed thoroughly and separately in the "The Inbox: > > > > > Kernel-ct.1405.mcz" > > > > > > > > thread. > > > > > > > > > > > > > > > > > > If you agree I'd really appreciate if your fix could be accepted > > > > > here in > > > > > > > > including the ProceedBlockCannotReturn exception I need to make it > > > > > work > > > > > > > > together with the code I'm presenting here. If we come up with a > > > > > better > > > > > > > > idea in the other discussion we can make amends here as well; I take > > > > > it as > > > > > > > > a patch, not a definitive solution :) > > > > > > > > > > > > > > > > > > Thanks again so much for all your suggestions and examples! It's > > > > > always > > > > > > > > a pleasure :) Plus - the Inbox Talk is priceless... I use it > > > > > exclusively > > > > > > > > now and look forward to any improvements you might come up in the > > > > > future ;) > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > > > > > > > > > > > > ^[^ Jaromir > > > > > > > > > -- > > > > > > > > > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > > > > > On 2021-08-22T16:49:10+02:00, christoph.thiede at > > > > > > > > student.hpi.uni-potsdam.de wrote: > > > > > > > > > > > > > > > > > > > Hi Jaromir, > > > > > > > > > > > > > > > > > > > > > Yes, I was wondering why I couldn't get rid of the duplication > > > > > and > > > > > > > > now I think it's because there really are two distinct unwind > > > > > semantics : > > > > > > > > one "light" for regular returns and one "heavy" for termination. > > > > > Both are > > > > > > > > very similar yet each require a slightly different behavior - that's > > > > > why > > > > > > > > the duality #runUntilErrorOrReturnFrom / > > > > > #runUnwindUntilErrorOrReturnFrom > > > > > > > > or #complete: / #complete:to: and #unwindTo: / #terminate. > > > > > > > > > > > > > > > > > > > > But they are still pretty similar ... Couldn't you just add some > > > > > extra > > > > > > > > parameter to these message? E.g., "Context runUntilErrorOrReturnFrom: > > > > > > > > aggressive"? I have the feeling that we could eliminate significant > > > > > > > > duplication that way by inserting some ifs and elses ... Duplicated > > > > > code > > > > > > > > very often tends to be harder to maintained. > > > > > > > > > > > > > > > > > > > > > With regards to #unwindTo: - I haven't tested it yet but I'm > > > > > > > > wondering whether it wouldn't have the same unwind problem with > > > > > non-local > > > > > > > > returns as the original #terminate and require a similar fix? > > > > > > > > > > > > > > > > > > > > Hm, do we need to implement both modi - (ii) and (iii) as > > > > > described in > > > > > > > > [1] - in Context >> #unwindTo: as well? Something like > > > > > > > > #unwindTo:aggressive:? > > > > > > > > > > > > > > > > > > > > > But in general - yes, any method/exception purposefully (or > > > > > not) > > > > > > > > written to create a loop will break this patch (I admit it is just a > > > > > patch > > > > > > > > really). I extracted it to #complete:to: to make #terminate clean; > > > > > this is > > > > > > > > a WIP; I wish there was a holistic solution to this - maybe checking > > > > > for > > > > > > > > exception recursion by default? :) > > > > > > > > > > > > > > > > > > > > Sounds better already, if feasible! But how would you detect > > > > > this? > > > > > > > > Unfortunately I've lost track of these infinite loops - could you > > > > > maybe > > > > > > > > point me to some concrete examples that lead to an infinite recursion > > > > > > > > without this special check? :-) > > > > > > > > > > > > > > > > > > > > One more question about your #runUnwindUntilErrorOrReturnFrom: > > > > > Are you > > > > > > > > maybe missing something like "cxt terminate" in the "No error was > > > > > raised" > > > > > > > > case? Just wondering. > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > Christoph > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > Sent from Squeak Inbox Talk > > > > > > > > > > > > > > > > > > > > On 2021-05-31T17:01:46-05:00, m at jaromir.net wrote: > > > > > > > > > > > > > > > > > > > > > Jaromir Matas wrote > > > > > > > > > > > > Hi All, > > > > > > > > > > > > I've sent an updated version of #teminate integrating > > > > > Christoph's > > > > > > > > solution > > > > > > > > > > > > of BlockCannotReturn recursion problem (in [1]), along with a > > > > > > > > battery of > > > > > > > > > > > > tests exploring termination of nested ensure and cascading > > > > > errors > > > > > > > > behavior > > > > > > > > > > > > (Debugger tests are for info and a final version can wait > > > > > until > > > > > > > > releasing > > > > > > > > > > > > Christoph's proposal in [2]). > > > > > > > > > > > > > > > > > > > > > > > > It's pretty much final, I hope... > > > > > > > > > > > > > > > > > > > > > > > > Any complaints about #terminate - please shout ;) > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > http://forum.world.st/The-Inbox-Kernel-ct-1405-mcz-td5129706.html > > > > > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > http://forum.world.st/The-semantics-of-halfway-executed-unwind-contexts-during-process-termination-tp5129800p5130110.html > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > > > > > > Here's the link: > > > > > > > > > > > > > > > > http://forum.world.st/The-Inbox-Kernel-jar-1414-mcz-td5130198.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- > > > > > > > > > > > ^[^ Jaromir > > > > > > > > > > > -- > > > > > > > > > > > Sent from: http://forum.world.st/Squeak-Dev-f45488.html > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
squeak-dev@lists.squeakfoundation.org