[squeak-dev] The Inbox: Kernel-jar.1414.mcz

mail at jaromir.net mail at jaromir.net
Wed Nov 17 21:05:16 UTC 2021


Hi Christoph,

just to keep things organized: I've addressed your comments in [1] and [2] where we discuss them. If you're happy with my reasoning behind ProceedBlockCannotReturn this changest is still ready to go :)

Thanks again for your comments,
best,

[1] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/217000.html
[2] http://lists.squeakfoundation.org/pipermail/squeak-dev/2021-November/216999.html

^[^ Jaromir
  --

Sent from Squeak Inbox Talk

On 2021-08-22T17:12:39+02:00, christoph.thiede at student.hpi.uni-potsdam.de wrote:

> Hi Jaromir,
> 
> just going to address this change from your patch, remaining review is following later:
> 
> > ----- Method: Context>>cannotReturn: (in category 'private-exceptions') -----
> > cannotReturn: result
> > 
> > + ����closureOrNil ifNotNil: [
> > + ��������| resumptionValue |
> > + ��������resumptionValue := self cannotReturn: result to: self home sender.
> > + ��������ProceedBlockCannotReturn new signal: 'This block has ended, continue with sender?'.
> > + ��������self pc > self endPC ifTrue: [
> > + ������������"This block has ended, continue with sender"
> > + ������������thisContext privSender: self sender].
> > + ��������^ resumptionValue].
> > - ����closureOrNil ifNotNil: [^ self cannotReturn: result to: self home sender].
> > ����Processor debugWithTitle: 'Computation has been terminated!!' translated full: false.!
> 
> 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.  Also consider any non-interactive scenario where a warning would make it even harder for the automator to perform the intended action. I would suggest adding an explaining comment in #cannotReturn:to: instead - something like: "By proceeding from this exception, the execution will be resumed."
> 
> Apart from that, the message text of your new warning is not correct if self pc <= self endPC. :-)
> 
> Best,
> Christoph
> 
> ---
> Sent from Squeak Inbox Talk
> 
> On 2021-05-31T21:39:39+00:00, commits at source.squeak.org wrote:
> 
> > A new version of Kernel was added to project The Inbox:
> > http://source.squeak.org/inbox/Kernel-jar.1414.mcz
> > 
> > ==================== Summary ====================
> > 
> > Name: Kernel-jar.1414
> > Author: jar
> > Time: 31 May 2021, 11:39:35.749017 pm
> > UUID: f300b29f-3257-e049-b139-8c05240cb97f
> > Ancestors: Kernel-jar.1413
> > 
> > finalize #terminate: solution:
> > - integrate Christoph's solution of BlockCannotReturn's infinite recursion problem
> > - solve a situation when a process gets terminated in the middle of unwinding another process
> > - supersede and replace Kernel-jar.1412 and Kernel-jar.1411
> > - complemented by: KernelTests-jar.406, KernelTests-jar.407, Tests-jar.466, ToolsTests-jar.105
> > 
> > 
> > Please remove Kernel-jar.1412, Kernel-jar.1411, KernelTests-jar.405, Tests-jar.465 from the Inbox.
> > 
> > 
> > Summary and discussion about the bugs and changes in #terminate: http://forum.world.st/Solving-multiple-termination-bugs-summary-amp-proposal-td5128285.html
> > 
> > =============== Diff against Kernel-jar.1413 ===============
> > 
> > Item was changed:
> > ----- Method: Context>>cannotReturn: (in category 'private-exceptions') -----
> > cannotReturn: result
> > 
> > + ����closureOrNil ifNotNil: [
> > + ��������| resumptionValue |
> > + ��������resumptionValue := self cannotReturn: result to: self home sender.
> > + ��������ProceedBlockCannotReturn new signal: 'This block has ended, continue with sender?'.
> > + ��������self pc > self endPC ifTrue: [
> > + ������������"This block has ended, continue with sender"
> > + ������������thisContext privSender: self sender].
> > + ��������^ resumptionValue].
> > - ����closureOrNil ifNotNil: [^ self cannotReturn: result to: self home sender].
> > ����Processor debugWithTitle: 'Computation has been terminated!!' translated full: false.!
> > 
> > Item was added:
> > + ----- Method: Context>>runUnwindUntilErrorOrReturnFrom: (in category 'private') -----
> > + runUnwindUntilErrorOrReturnFrom: aSender 
> > + ����"ASSUMES aSender is a sender of self. Execute self's stack until aSender returns or an unhandled exception is raised. Return a pair containing the new top context and a possibly nil exception. The exception is not nil if it was raised before aSender returned and it was not handled. The exception is returned rather than openning the debugger, giving the caller the choice of how to handle it."
> > + ����"Self is run by jumping directly to it (the active process abandons thisContext and executes self). However, before jumping to self we insert an ensure block under aSender that jumps back to thisContext when evaluated. We also insert an exception handler under aSender that jumps back to thisContext when an unhandled exception is raised. In either case, the inserted ensure and exception handler are removed once control jumps back to thisContext."
> > + 
> > + ����| error ctxt here topContext |
> > + ����here := thisContext.
> > + 
> > + ����"Insert ensure and exception handler contexts under aSender"
> > + ����error := nil.
> > + ����ctxt := aSender insertSender: (Context
> > + ��������contextOn: UnhandledError do: [:ex |
> > + ������������error ifNil: [
> > + ����������������error := ex exception.
> > + ����������������topContext := thisContext.
> > + ����������������here jump.
> > + ����������������ex signalerContext restart "re-signal the error when jumped back"]
> > + ������������ifNotNil: [ex pass]
> > + ��������]).
> > + ����ctxt := ctxt insertSender: (Context
> > + ��������contextEnsure: [error ifNil: [
> > + ����������������topContext := thisContext.
> > + ����������������here jump]
> > + ��������]).
> > + ����self jump. "Control jumps to self"
> > + 
> > + ����"Control resumes here once above ensure block or exception handler is executed"
> > + ����^ error ifNil: [
> > + ��������"No error was raised, return the sender of the above ensure context (see Note 1)"
> > + ��������{ctxt sender. nil}
> > + 
> > + ����] ifNotNil: [
> > + ��������"Error was raised, remove inserted above contexts then return signaler context"
> > + ��������aSender terminateTo: ctxt sender. "remove above ensure and handler contexts"
> > + ��������{topContext. error}
> > + ����]
> > + 
> > + "Note 1: It doesn't matter 'ctxt sender' is not a proper top context because #terminate will use it only as a starting point in the search for the next unwind context and computation will never return here. Removing the inserted ensure context (i.e. ctxt) by stepping until popped (as in #runUntilErrorOrReturnFrom:) when executing non-local returns is not applicable here and would fail testTerminationDuringNestedUnwindWithReturn1 through 4."!
> > 
> > Item was added:
> > + Warning subclass: #ProceedBlockCannotReturn
> > + ����instanceVariableNames: ''
> > + ����classVariableNames: ''
> > + ����poolDictionaries: ''
> > + ����category: 'Kernel-Exceptions'!
> > 
> > Item was added:
> > + ----- Method: Process>>complete:to: (in category 'private') -----
> > + complete: topContext to: aContext 
> > + ����"Run topContext on behalf of self on topContext's stack until aContext is popped or an unhandled 
> > + ����error is raised. Return self's new top context. Note: topContext must be a stack top context.
> > + ����This method is meant to be called primarily by Process>>#terminate."
> > + 
> > + ����| pair top error |
> > + ����pair := Processor activeProcess
> > + ����������������evaluate: [topContext runUnwindUntilErrorOrReturnFrom: aContext]
> > + ����������������onBehalfOf: self.
> > + ����top := pair first.
> > + ����error := pair second.
> > + ����"If an error was detected jump back to the debugged process and re-signal the error;
> > + ����some errors may require a special care - see notes below."
> > + ����error ifNotNil: [
> > + ��������error class == ProceedBlockCannotReturn ifTrue: [^top]. "do not jump back"
> > + ��������error class == MessageNotUnderstood ifTrue: [error initialize]. "reset reachedDefaultHandler"
> > + ��������top jump].
> > + ����^top
> > + 
> > + "Note 1: To prevent an infinite recursion of the MessageNotUnderstood error, reset reachedDefaultHandler before jumping back; this will prevent #doesNotUnderstand: from resending the unknown message.
> > + Note 2; To prevent returning from the BlockCannotReturn error, do not jump back when ProceedBlockCannotReturn warning has been raised."!
> > 
> > Item was changed:
> > ----- Method: Process>>terminate (in category 'changing process state') -----
> > terminate 
> > ����"Stop the process that the receiver represents forever.
> > ���� Unwind to execute pending ensure:/ifCurtailed: blocks before terminating.
> > ���� If the process is in the middle of a critical: critical section, release it properly."
> > 
> > + ����| oldList top ctxt outerMost newTop unwindBlock |
> > + ����"If terminating the active process, suspend it first and terminate it as a suspended process."
> > - ����| ctxt unwindBlock oldList outerMost |
> > ����self isActiveProcess ifTrue: [
> > - ��������"If terminating the active process, suspend it first and terminate it as a suspended process."
> > ��������[self terminate] fork.
> > ��������^self suspend].
> > 
> > + ����[] ensure: ["Execute termination as an unwind block to ensure it completes even if terminated;
> > + ��������see testTerminateInTerminate."
> > + ��������"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 ifNil: [^self]. "self is already terminated"
> > + ��������"Release any method marked with the <criticalSection> pragma.
> > + ��������The argument is whether the process is runnable."
> > + ��������self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]).
> > - ����"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]).
> > 
> > + ��������top := suspendedContext.
> > + ��������suspendedContext := nil. "disable this process while running its stack in active process below"
> > ��������"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; nested 
> > + ��������unwind blocks will be completed in the process (see testTerminationDuringUnwind, testNestedUnwind). 
> > + ��������Note: Halfway-through blocks have already set the complete variable (tempAt: 2) in their defining
> > + ��������#ensure:/#ifCurtailed contexts from nil to true; we'll search for the bottom-most one.
> > + ��������Note: #findNextUnwindContextUpTo: starts searching from the receiver's sender but the receiver 
> > + ��������itself may be an unwind context (see testTerminateEnsureAsTopContext)."
> > + ��������ctxt := top.
> > + ��������ctxt isUnwindContext ifFalse: [ctxt := ctxt findNextUnwindContextUpTo: nil].
> > + ��������[ctxt isNil] whileFalse: [
> > + ������������(ctxt tempAt:2) ifNotNil: [
> > + ����������������outerMost := ctxt].
> > + ������������ctxt := ctxt findNextUnwindContextUpTo: nil].
> > + ��������outerMost ifNotNil: [newTop := self complete: top to: outerMost].
> > - ��������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);
> > - ��������������������return, if the execution stack reached its bottom (e.g. in case of non-local returns)."
> > - ��������������������(suspendedContext := outerMost sender) ifNil: [^self]]]. 
> > 
> > + ��������"By now no halfway-through unwind blocks are on the stack. Create a new top context for each 
> > + ��������pending unwind block (tempAt: 1) and execute it on the unwind block's stack. 
> > + ��������Note: using #value instead of #complete:to: would lead to incorrect evaluation of non-local returns.
> > + ��������Note: newTop sender points to the former outerMost sender, i.e. the next unexplored context."
> > + ��������ctxt := newTop ifNil: [top] ifNotNil: [newTop sender].
> > - ��������"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.
> > + ����������������top := unwindBlock asContextWithSender: ctxt.
> > + ����������������self complete: top to: top].
> > + ������������ctxt := ctxt findNextUnwindContextUpTo: nil]
> > + ����]!
> > - ����������������"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].
> > - 
> > - ��������"Reset the context's pc and sender to nil for the benefit of isTerminated."
> > - ��������suspendedContext terminate]!
> > 
> > 
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210822/7e164fde/attachment.html>
> 
> 


More information about the Squeak-dev mailing list