<div dir="ltr">Hi Chris,<div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 1, 2018 at 6:55 PM, Chris Muller <span dir="ltr"><<a href="mailto:asqueaker@gmail.com" target="_blank">asqueaker@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">My image just locked after it tried to unwind normally from a network<br>
timeout. I had to kill it from the OS.<br>
<br>
Eliot would you guide me how to get my trunk image rolled back to a<br>
state without this fix so I can try to get back to where I was to see<br>
if its related?<br>
<br>
Should I just load Kernel-eem.1184, an earlier one,<br>
or something else?<br></blockquote><div><br></div><div>1182 is the last one before any of the critical: termination changes. </div><div>1183 has the critical: termination changes but doesn't change Semaphore>>critical: to the simple version.</div><div>1184 has the simpler Semaphore>>critical:</div><div><br></div><div>I would like to have a look at this image. My changes look good to me and others. One possible issue is that the send of releaseCriticalSection: in Process>>terminate considers a double attempt at terminating a process (for example sending terminate within an unwind block that is being run during terminate) an attempt to terminate a live process. That could be a mistake, and instead the send should only consider the process live if suspend answers a LinkedList. So you could experiment with changing the line</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]).<br></div><div><br></div><div>to</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> self releaseCriticalSection: oldList class == LinkedList.<br></div><div><br></div><div>But if it is possible to reproduce this lookup then I want to have a look and try and see what is going on.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
<br>
<br>
On Fri, Jul 27, 2018 at 1:08 PM, <<a href="mailto:commits@source.squeak.org">commits@source.squeak.org</a>> wrote:<br>
> A new version of Kernel was added to project The Inbox:<br>
> <a href="http://source.squeak.org/inbox/Kernel-eem.1185.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/<wbr>inbox/Kernel-eem.1185.mcz</a><br>
><br>
> ==================== Summary ====================<br>
><br>
> Name: Kernel-eem.1185<br>
> Author: eem<br>
> Time: 27 July 2018, 11:08:12.641836 am<br>
> UUID: 7296ad00-708d-4cef-b6bc-<wbr>ceb24d897a70<br>
> Ancestors: Kernel-eem.1184<br>
><br>
> In releaseCriticalSection: use isUnwindContext instead of the more fragile context selector == #ensure:. Now it will handle ifCurtailed: too. Thanks Tobias!<br>
><br>
> Reformat Process>>#terminate. It was baaad.<br>
><br>
> =============== Diff against Kernel-eem.1184 ===============<br>
><br>
> Item was changed:<br>
> ----- Method: Process>><wbr>releaseCriticalSection: (in category 'private') -----<br>
> releaseCriticalSection: runnable<br>
> "Figure out if we are terminating a process that is in the ensure: block of a critical section.<br>
> In this case, if the block has made progress, pop the suspendedContext so that we leave the<br>
> ensure: block inside the critical: without signaling the semaphore/exiting the primitive section,<br>
> since presumably this has already happened. But if it hasn't made progress but is beyond the<br>
> + wait (which we can tell by the oldList being one of the runnable lists, i.e. a LinkedList, not a<br>
> - wait (which we can tell my the oldList being one of the runnable lists, i.e. a LinkedList, not a<br>
> Semaphore or Mutex, et al), then the ensure: block needs to be run."<br>
> | selectorJustSent |<br>
> (suspendedContext method pragmaAt: #criticalSection) ifNil: [^self].<br>
> selectorJustSent := suspendedContext selectorJustSentOrSelf.<br>
><br>
> "Receiver and/or argument blocks of ensure: in Semaphore>>critical: or Mutex>>#critical:"<br>
> suspendedContext isClosureContext ifTrue:<br>
> + [suspendedContext sender isUnwindContext ifTrue:<br>
> - [suspendedContext sender selector == #ensure: ifTrue:<br>
> [| notWaitingButMadeNoProgress |<br>
> "Avoid running the ensure: block twice, popping it if it has already been run. If runnable<br>
> but at the wait, leave it in place. N.B. No need to check if the block receiver of ensure: has<br>
> not started to run (via suspendedContext pc = suspendedContext startpc) because ensure:<br>
> uses valueNoContextSwitch, and so there is no suspension point before the wait."<br>
> notWaitingButMadeNoProgress :=<br>
> runnable<br>
> and: [selectorJustSent == #wait<br>
> and: [suspendedContext sender selectorJustSentOrSelf == #valueNoContextSwitch]].<br>
> notWaitingButMadeNoProgress ifFalse:<br>
> [suspendedContext := suspendedContext home]].<br>
> ^self].<br>
><br>
> "Either Semaphore>>critical: or Mutex>>#critical:. Is the process still blocked? If so, nothing further to do."<br>
> runnable ifFalse: [^self].<br>
><br>
> "If still at the wait the ensure: block has not been activated, so signal to restore."<br>
> selectorJustSent == #wait ifTrue:<br>
> [suspendedContext receiver signal].<br>
><br>
> "If still at the lock primitive and the lock primitive just acquired ownership (indicated by it answering false)<br>
> then the ensure block has not been activated, so explicitly primitiveExitCriticalSection to unlock."<br>
> (selectorJustSent == #primitiveEnterCriticalSection<br>
> or: [selectorJustSent == #<wbr>primitiveTestAndSetOwnershipOf<wbr>CriticalSection]) ifTrue:<br>
> [(suspendedContext stackPtr > 0<br>
> and: [suspendedContext top == false]) ifTrue:<br>
> [suspendedContext receiver primitiveExitCriticalSection]]<wbr>!<br>
><br>
> Item was changed:<br>
> ----- Method: Process>>terminate (in category 'changing process state') -----<br>
> terminate<br>
> "Stop the process that the receiver represents forever.<br>
> Unwind to execute pending ensure:/ifCurtailed: blocks before terminating.<br>
> If the process is in the middle of a critical: critical section, release it properly."<br>
><br>
> | ctxt unwindBlock oldList |<br>
> + self isActiveProcess ifTrue:<br>
> + [ctxt := thisContext.<br>
> + [ctxt := ctxt findNextUnwindContextUpTo: nil.<br>
> + ctxt ~~ nil] whileTrue:<br>
> + [(ctxt tempAt: 2) ifNil:<br>
> + ["N.B. Unlike Context>>unwindTo: we do not set complete (tempAt: 2) to true."<br>
> + unwindBlock := ctxt tempAt: 1.<br>
> + thisContext terminateTo: ctxt.<br>
> + unwindBlock value]].<br>
> - self isActiveProcess ifTrue: [<br>
> - ctxt := thisContext.<br>
> - [ ctxt := ctxt findNextUnwindContextUpTo: nil.<br>
> - ctxt isNil<br>
> - ] whileFalse: [<br>
> - (ctxt tempAt: 2) ifNil:[<br>
> - ctxt tempAt: 2 put: nil.<br>
> - unwindBlock := ctxt tempAt: 1.<br>
> - thisContext terminateTo: ctxt.<br>
> - unwindBlock value].<br>
> - ].<br>
> thisContext terminateTo: nil.<br>
> self suspend.<br>
> + "If the process is resumed this will provoke a cannotReturn: error.<br>
> + Would self debug: thisContext title: 'Resuming a terminated process' be better?"<br>
> + ^self].<br>
> - ] ifFalse:[<br>
> - "Always suspend the process first so it doesn't accidentally get woken up.<br>
> - N.B. If oldList is a LinkedList then the process is runnable. If it is a Semaphore/Mutex et al<br>
> - then the process is blocked, and if it is nil then the process is already suspended."<br>
> - oldList := self suspend.<br>
> - suspendedContext ifNotNil:<br>
> - ["Release any method marked with the <criticalSection> pragma.<br>
> - The argument is whether the process is runnable."<br>
> - self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]).<br>
><br>
> + "Always suspend the process first so it doesn't accidentally get woken up.<br>
> + N.B. If oldList is a LinkedList then the process is runnable. If it is a Semaphore/Mutex et al<br>
> + then the process is blocked, and if it is nil then the process is already suspended."<br>
> + oldList := self suspend.<br>
> + suspendedContext ifNotNil:<br>
> + ["Release any method marked with the <criticalSection> pragma.<br>
> + The argument is whether the process is runnable."<br>
> + self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]).<br>
> - "If terminating a process halfways through an unwind, try to complete that unwind block first."<br>
> - (suspendedContext findNextUnwindContextUpTo: nil) ifNotNil:<br>
> - [:outer|<br>
> - (suspendedContext findContextSuchThat:[:c| c closure == (outer tempAt: 1)]) ifNotNil:<br>
> - [:inner| "This is an unwind block currently under evaluation"<br>
> - suspendedContext runUntilErrorOrReturnFrom: inner]].<br>
><br>
> + "If terminating a process halfways through an unwind, try to complete that unwind block first."<br>
> + (suspendedContext findNextUnwindContextUpTo: nil) ifNotNil:<br>
> + [:outer|<br>
> + (suspendedContext findContextSuchThat:[:c| c closure == (outer tempAt: 1)]) ifNotNil:<br>
> + [:inner| "This is an unwind block currently under evaluation"<br>
> + suspendedContext runUntilErrorOrReturnFrom: inner]].<br>
> +<br>
> + ctxt := self popTo: suspendedContext bottomContext.<br>
> + ctxt == suspendedContext bottomContext ifFalse:<br>
> + [self debug: ctxt title: 'Unwind error during termination'].<br>
> + "Set the context to its endPC for the benefit of isTerminated."<br>
> + ctxt pc: ctxt endPC]!<br>
> - ctxt := self popTo: suspendedContext bottomContext.<br>
> - ctxt == suspendedContext bottomContext ifFalse:<br>
> - [self debug: ctxt title: 'Unwind error during termination'].<br>
> - "Set the context to its endPC for the benefit of isTerminated."<br>
> - ctxt pc: ctxt endPC]]!<br>
><br>
><br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div></div>