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: 'allcleaned 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