<div dir="ltr">Hi David, Hi Bert, Clément, Juan, Levente and Marcus, Hi Anyone else with strong experience in the VM with processes,<div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 23, 2018 at 7:38 PM, David T. Lewis <span dir="ltr"><<a href="mailto:lewis@mail.msen.com" target="_blank">lewis@mail.msen.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">Semaphore seems like a rather basic thing that should work correctly in<br>
any Squeak image. The tests do not pass in trunk any more.<br>
<br>
Specifically, SemaphoreTest>><wbr>testSemaInCriticalWait fails in trunk, but passes<br>
in the earlier Squeak 4.6 / 5.0 images.<br>
<br>
Is this a real problem? Does it need to be fixed for the 5.2 release?<br></blockquote><div><br></div><div>Yes.  Yes.  And it needs to be fixed in Pharo too.  I know this message will strike you as TL;DR, but please, especially if you're Bert, Clément, Juan, Levente or Marcus, read this carefully.  It's quite important.  And below I'll present the Squeak code but will work with Clément and Marcus to implement semantically equivalent Pharo code asap.</div><div><br></div><div>And apologies in advance for the repetitious nature of this message.  It is better that I am precise than I am brief and anyone miss anything.  This is an old problem and it will be nice if I've fixed it, but I could easily have missed something; this problem having been around for decades.  OK...</div><div><br></div><div><br></div><div>This is an old problem which boiled down to there being no way to determine by looking at a process's suspendedContext whether a process is either waiting on a Semaphore or Mutex or is no longer waiting, but has made no progress because it is at a lower priority than runnable processes and so has not got a chance to run yet.</div><div><br></div><div>So in</div><div>    | s |</div><div>    s := Semaphore new.</div><div>    ...</div><div>    s wait</div><div>    ...</div><div><br></div><div>if we look at the context containing the wait its pc will be the same whether the process is blocked, waiting on the semaphore, or whether the semaphore has been signalled but the process has not been able to proceed because it is of lower priority than runnable processes and so can make no progress.  This caused problems for code such as this:</div><div><br></div><div>Semaphore>>critical: mutuallyExcludedBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>^mutuallyExcludedBlock ensure: [self signal]</div><div><br></div><div>because the ensure: won't be entered if higher priority runnable processes are preventing it from running.</div><div><br></div><div>And for code such as this:</div><div><br></div><div>Semaphore>>critical: mutuallyExcludedBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   ^[</span>self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>mutuallyExcludedBlock value]</div><div><span style="white-space:pre">       </span><span style="white-space:pre">     </span>ensure: [self signal]</div><div><br></div><div>because if the process is terminated when the semaphore has not been signalled (i.e. the process is blocked in the wait), Process>>terminate will run the ensure: block anyway, resulting in the Semaphore getting an extra signal.</div><div><br></div><div>This occupied Andreas and I at Qwaq, and we didn't solve it.  We developed Mutex as a more efficient version of Monitor, but this is also subject so the same problem.  We did change the definition of ensure: so that it is not a suspension point, by adding valueNoContextSwitch[:]</div><div><br></div><div><div>BlockClosure>>ensure: aBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span>"Evaluate a termination block after evaluating the receiver, regardless of</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span> whether the receiver's evaluation completes.  N.B.  This method is *not*</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span> implemented as a primitive.  Primitive 198 always fails.  The VM uses prim</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span> 198 in a context's method as the mark for an ensure:/ifCurtailed: activation."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>| complete returnValue |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span><primitive: 198></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>returnValue := self valueNoContextSwitch.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>complete ifNil:[</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">               </span>complete := true.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">              </span>aBlock value.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>^ returnValue</div></div><div><br></div><div>This means that we don't have to deal with suspensions here (marked with !!!)</div><div><br></div><div>I now understand how to distinguish between the two cases, between blocking and not blocked but no progress.  Process>>suspend answers the list the Process was on when it was suspended.  If the process is already suspended Process>>suspend answers nil.  If the process is waiting on a Semaphore or a Mutex, Process>>suspend answers the Semaphore or Mutex. And if the process is runnable then Process>>suspend answers the process's run list (a LinkedList in ProcessorScheduler's quiescentProcessLists array corresponding to the process's priority).</div><div><br></div><div>So Process>>#terminate can distinguish between #wait or #primitiveEnterCriticalSection or #primitiveTestAndSetOwnershipOfCriticalSection being blocked, or being unblocked but having made no progress due to too low a priority.  We do so by testing the class of the result of suspending the process.  If it is a LinkedList, the process has past the #wait or #primitiveEnterCriticalSection but has made no progress due to too low a priority.</div><div><br></div><div>The version of Process>>#terminate I'm about to commit deals with several cases.  Let me present the cases first.  There are three versions of Semaphore>>#critical: to handle, and one version of Mutex>>critical: and Mutex>>#critical:ifLocked:.</div><div><br></div><div>The two basic versions of Semaphore>>critical: are</div><div><br></div><div>V1</div><div><div>critical: mutuallyExcludedBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>"Evaluate mutuallyExcludedBlock only if the receiver is not currently in</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>the process of running the critical: message. If the receiver is, evaluate</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>mutuallyExcludedBlock after the other critical: message is finished."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span><criticalSection></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>^mutuallyExcludedBlock ensure: [self signal]</div></div><div><br></div><div>V2</div><div><div>critical: mutuallyExcludedBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>"Evaluate mutuallyExcludedBlock only if the receiver is not currently in</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>the process of running the critical: message. If the receiver is, evaluate</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>mutuallyExcludedBlock after the other critical: message is finished."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span><criticalSection></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>^[self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>   mutuallyExcludedBlock value]</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">               </span>ensure: [self signal]</div></div><div><br></div><div>and Juan's safer version is (after I added the criticalSection pragma)</div><div><br></div><div>V3</div><div><div>critical: mutuallyExcludedBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>"Evaluate mutuallyExcludedBlock only if the receiver is not currently in</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>the process of running the critical: message. If the receiver is, evaluate</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>mutuallyExcludedBlock after the other critical: message is finished."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     <criticalSection></span></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>| caught |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>"We need to catch eventual interruptions very carefully. </div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>The naive approach of just doing, e.g.,:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">               </span>self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>aBlock ensure:[self signal].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>will fail if the active process gets terminated while in the wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>However, the equally naive:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>[self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>aBlock value] ensure:[self signal].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>will fail too, since the active process may get interrupted while</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>entering the ensured block and leave the semaphore signaled twice.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>To avoid both problems we make use of the fact that interrupts only</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>occur on sends (or backward jumps) and use an assignment (bytecode)</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>right before we go into the wait primitive (which is not a real send and</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span>therefore not interruptable either)."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>caught := false.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span>^[</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>caught := true.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                </span>self wait.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>mutuallyExcludedBlock value</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>] ensure: [ caught ifTrue: [self signal] ]</div></div><div><br></div><div>and the Mutex>>critical:'s are</div><div><br></div><div><div>critical: aBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>"Evaluate aBlock protected by the receiver."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><criticalSection></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>^self primitiveEnterCriticalSection</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>ifTrue: [aBlock value]</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">         </span>ifFalse: [aBlock ensure: [self primitiveExitCriticalSection]]</div></div><div><br></div><div><div>critical: aBlock ifLocked: lockedBlock</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>"Answer the evaluation of aBlock protected by the receiver.  If it is already in a critical</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span> section on behalf of some other process answer the evaluation of lockedBlock."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span><criticalSection></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>^self primitiveTestAndSetOwnershipOfCriticalSection</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>ifNil: [lockedBlock value]</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>ifNotNil:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                      </span>[:alreadyOwner|</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                        </span> alreadyOwner</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                          </span>ifTrue: [aBlock value]</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                         </span>ifFalse: [aBlock ensure: [self primitiveExitCriticalSection]]]</div></div><div><br></div><div>primitiveEnterCriticalSection answers false if the Mutex was unowned, and true if it was already owned by the active process.  It blocks otherwise.  primitiveTestAndSetOwnershipOfCriticalSection answers false if the Mutex was unowned, true if it was already owned by the active process, and nil if owned by some other process.<br></div><div><br></div><div>So we want Process>>#terminate to correctly release the semaphores and mutexes no matter where in these methods they are.  We don't have to worry if the process is within the block argument to a critical: itself, only if it is actually within the critical: method or a block within it. If it is already within the block argument to critical: then Process>>#terminate's unwind handling will unwind things correctly.  Taking Juan's version of Semaphore>>#critical: above, the key issue is whether the process being terminated is blocked on the wait, not blocked but still stuck at the wait, or at the start of the block argument to ensure:.</div><div><br></div><div>I have extracted the processing into Process>>releaseCriticalSection:, so now Process>>terminate reads</div><div><br></div><div><div>terminate </div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>"Stop the process that the receiver represents forever.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span> Unwind to execute pending ensure:/ifCurtailed: blocks before terminating.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span> If the process is in the middle of a critical: critical section, release it properly."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>| ctxt unwindBlock oldList |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>self isActiveProcess ifTrue: [</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">         </span>ctxt := thisContext.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">           </span>[<span class="gmail-Apple-tab-span" style="white-space:pre">     </span>ctxt := ctxt findNextUnwindContextUpTo: nil.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                   </span>ctxt isNil</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>] whileFalse: [</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                        </span>(ctxt tempAt: 2) ifNil:[</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                               </span>ctxt tempAt: 2 put: nil.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                               </span>unwindBlock := ctxt tempAt: 1.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                         </span>thisContext terminateTo: ctxt.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                         </span>unwindBlock value].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>thisContext terminateTo: nil.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">          </span>self suspend.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>] ifFalse:[</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>"Always suspend the process first so it doesn't accidentally get woken up.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span> N.B. If oldList is a LinkedList then the process is runnable. If it is a Semaphore/Mutex et al</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                </span> then the process is blocked, and if it is nil then the process is already suspended."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">            </span>oldList := self suspend.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">               </span>suspendedContext ifNotNil:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                     </span>["Release any method marked with the <criticalSection> pragma.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                      </span>  The argument is whether the process is runnable."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                      </span> self releaseCriticalSection: (oldList isNil or: [oldList class == LinkedList]).</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                      </span>"If terminating a process halfways through an unwind, try to complete that unwind block first."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                      </span>(suspendedContext findNextUnwindContextUpTo: nil) ifNotNil:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                            </span>[:outer|</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                               </span>(suspendedContext findContextSuchThat:[:c| c closure == (outer tempAt: 1)]) ifNotNil:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                  </span>[:inner| "This is an unwind block currently under evaluation"</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                        </span>suspendedContext runUntilErrorOrReturnFrom: inner]].</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                  </span>ctxt := self popTo: suspendedContext bottomContext.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                    </span>ctxt == suspendedContext bottomContext ifFalse:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                </span>[self debug: ctxt title: 'Unwind error during termination'].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                   </span>"Set the context to its endPC for the benefit of isTerminated."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                      </span>ctxt pc: ctxt endPC]]</div></div><div><br></div><div>In implementing releaseCriticalSection: we need to know which selector a context has just sent.  selectorJustSentOrSelf is implemented in Squeak as</div><div><br></div><div>InstructionStream>>selectorJustSentOrSelf</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>"If this instruction follows a send, answer the send's selector, otherwise answer self."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>| method |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>method := self method.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>^method encoderClass selectorToSendOrItselfFor: self in: method at: self previousPc</div><div><br></div><div>c.f.</div><div><br></div><div><div>InstructionStream>>selectorToSendOrSelf</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>"If this instruction is a send, answer the selector, otherwise answer self."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>| method |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>method := self method.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>^method encoderClass selectorToSendOrItselfFor: self in: method at: pc</div></div><div><br></div><div>Now we can implement Process>>#releaseCriticalSection:</div><div><br></div><div><div>releaseCriticalSection: runnable</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>"Figure out if we are terminating a process that is in the ensure: block of a critical section.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span> In this case, if the block has made progress, pop the suspendedContext so that we leave the</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span> ensure: block inside the critical: without signaling the semaphore/exiting the primitive section,</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span> since presumably this has already happened.  But if it hasn't made progress but is beyond the</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span> wait (which we can tell my the oldList being one of the runnable lists, i.e. a LinkedList, not a</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span> Semaphore or Mutex, et al), then the ensure: block needs to be run."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>| selectorJustSent |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>(suspendedContext method pragmaAt: #criticalSection) ifNil: [^self].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>selectorJustSent := suspendedContext selectorJustSentOrSelf.</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span>"Receiver and/or argument blocks of ensure: in Semaphore>>critical: or Mutex>>#critical:"</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>suspendedContext isClosureContext ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">              </span>[suspendedContext sender selector == #ensure: ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                  </span>[| notWaitingButMadeNoProgress |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>"Avoid running the ensure: block twice, popping it if it has already been run. If runnable</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                        </span> but at the wait, leave it in place. N.B. No need to check if the block receiver of ensure: has</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                        </span> not started to run (via suspendedContext pc = suspendedContext startpc) because ensure:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span> uses valueNoContextSwitch, and so there is no suspension point before the wait."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                 </span> notWaitingButMadeNoProgress :=</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                </span>runnable</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                               </span>and: [selectorJustSent == #wait</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                </span>and: [suspendedContext sender selectorJustSentOrSelf == #valueNoContextSwitch]].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span> notWaitingButMadeNoProgress ifFalse:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                          </span>[suspendedContext := suspendedContext home]].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">          </span> ^self].</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>"Either Semaphore>>critical: or Mutex>>#critical:.  Is the process still blocked?  If so, nothing further to do."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>runnable ifFalse: [^self].</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>"If still at the wait the ensure: block has not been activated, so signal to restore."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span>selectorJustSent == #wait ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">              </span>[suspendedContext receiver signal].</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre">   </span>"If still at the lock primitive and the lock primitive just acquired ownership (indicated by it answering false)</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span> then the ensure block has not been activated, so explicitly primitiveExitCriticalSection to unlock."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>(selectorJustSent == #primitiveEnterCriticalSection</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span> or: [selectorJustSent == #primitiveTestAndSetOwnershipOfCriticalSection]) ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>[(suspendedContext stackPtr > 0</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>  and: [suspendedContext top == false]) ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>[suspendedContext receiver primitiveExitCriticalSection]]</div></div><div><br></div><div><br></div><div>Let's go through it line by line.  First, runnable is an argument, determined in Process>>#terminate.  One could invoke it with</div><div><br></div><div>    self releaseCriticalSection: oldList class == LinkedList<br></div><div><br></div><div>but this means that an already suspended process is assumed to be not runnable, which makes it tricky to debug the Process>>#terminate method.  One has to assign to oldList while stepping though the method.  I've chosen safety, assuming that the process is still runnable if suspend answers nil, its simply being debugged.</div><div><br></div><div>Then we're only interested in <criticalSection> marked methods se we return if there's no such pragma.</div><div><br></div><div>Then we deal with blocks in these methods.  One issue here is to avoid running the ensure: block twice if it is already being evaluated.  The other is to run it if it is stalled and has yet to be run.</div><div><br></div><div>So if</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>suspendedContext isClosureContext ifTrue:</div></div><div><br></div><div>we're in the ensure: receiver or argument blocks in any <criticalSection> marked method, i.e. Semaphore>>critical: and Mutex>>critical:[ifLocked:].  If wait was just sent then we're in the ensure: receiver block of Semaphore>>critical: (V2 & V3 above) and the issue is whether the process is blocked or is unblocked and has made no progress. If blocked then nothing needs to be done; the ensure: block is discarded and the stack cut back to the critical: activation.  If progress has been made then nothing needs to be done (in fact we can't be in this state; the ensure: receiver will have started evaluating the critical: block argument).  If unblocked, but no progress has been made, do /not/ discard the unwind block and it will be run in Process>>#terminate when this method returns.  Hence...</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre">              </span>[suspendedContext sender selector == #ensure: ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                  </span>[| notWaitingButMadeNoProgress |</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>"Avoid running the ensure: block twice, popping it if it has already been run. If runnable</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                        </span> but at the wait, leave it in place. N.B. No need to check if the block receiver of ensure: has</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                        </span> not started to run (via suspendedContext pc = suspendedContext startpc) because ensure:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span> uses valueNoContextSwitch, and so there is no suspension point before the wait."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                 </span> notWaitingButMadeNoProgress :=</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                </span>runnable</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                               </span>and: [selectorJustSent == #wait</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                                </span>and: [suspendedContext sender selectorJustSentOrSelf == #valueNoContextSwitch]].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span> notWaitingButMadeNoProgress ifFalse:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                          </span>[suspendedContext := suspendedContext home]].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">          </span> ^self].</div></div><div><br></div><div>Now we're left with the simpler version of Semaphore>>critical: (V1 above) and the two Mutex methods Mutex>>#critical:[ifLocked:].  Here the only state we have to worry about is that the process is unblocked but has made no progress.  If not runnable the process is still blocked and we can simply return.</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span>"Either Semaphore>>critical: or Mutex>>#critical:.  Is the process still blocked?  If so, nothing further to do."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">      </span>runnable ifFalse: [^self].</div><div><br></div></div><div>If #wait was just sent the process is in Semaphore>>#critical: and, because ensure: has not been sent we signal explicitly to restore the signal count:</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre">        </span>"If still at the wait the ensure: block has not been activated, so signal to restore."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span>selectorJustSent == #wait ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">              </span>[suspendedContext receiver signal].</div></div><div><br></div><div>If either of primitiveEnterCriticalSection or primitiveTestAndSetOwnershipOfCriticalSection have just been sent then either the Mutex is already owned, in which case the ensure block is elsewhere in the colder part of the stack, or has just been owned, and because ensure: has not been sent we unlock explicitly to release the Mutex:</div><div><br></div><div><div><span class="gmail-Apple-tab-span" style="white-space:pre">       </span>"If still at the lock primitive and the lock primitive just acquired ownership (indicated by it answering false)</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">  </span> then the ensure block has not been activated, so explicitly primitiveExitCriticalSection to unlock."</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">     </span>(selectorJustSent == #primitiveEnterCriticalSection</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">    </span> or: [selectorJustSent == #primitiveTestAndSetOwnershipOfCriticalSection]) ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>[(suspendedContext stackPtr > 0</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">             </span>  and: [suspendedContext top == false]) ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre">                       </span>[suspendedContext receiver primitiveExitCriticalSection]]</div></div><div><br></div><div>So Pharoers can you read and say whether you think this is sane or not?  If so, then we can kibbutz to write the Pharo version.</div><div><br></div><div>Squeakers can you review Kernel-eem.1183 & Kernel-eem.1184 in the inbox?  Kernel-eem.1183 includes the fix as described above.  Kernel-eem.1184 reverts Semaphore>>#critical: to V1 above.</div><div><br></div><div><br></div><div>P.S. Looking at V1 above it seems to me that there is an issue if the process is preempted in ensure: before sending valueNoContextSwitch:.  I'll try and write a test that advances a process to that precise point.  If that test fails I think we have to use V2 or V3, and V2 is clearly preferable.</div><div><br></div></div><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>