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