<div dir="ltr"><span style="color:rgb(0,0,0);font-size:12.800000190734863px">Hi Chris,</span><div style="color:rgb(0,0,0);font-size:12.800000190734863px"><br></div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"> this won't work as written. Suspending the active process within a critical: block prevents the critical: block from completing and releasing the lock. So with the changes as written another debugger won't open when one is already open.</div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"><br></div><div style="color:rgb(0,0,0);font-size:12.800000190734863px">One should never do this:</div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"><br></div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"> aMutexOrSemaphore critical:</div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"> [self doStuff.</div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"> Processor activeProcess suspend]</div><div style="color:rgb(0,0,0);font-size:12.800000190734863px"><br></div><div style="color:rgb(0,0,0);font-size:12.800000190734863px">I'm revising these cases to pull the suspend out of the critical block.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 4, 2017 at 1:52 PM, <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Chris Muller uploaded a new version of Morphic to project The Trunk:<br>
<a href="http://source.squeak.org/trunk/Morphic-cmm.1333.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/<wbr>trunk/Morphic-cmm.1333.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: Morphic-cmm.1333<br>
Author: cmm<br>
Time: 4 April 2017, 3:52:04.9926 pm<br>
UUID: 64767f0b-c6c4-4c32-97a9-<wbr>d122c30222d5<br>
Ancestors: Morphic-eem.1332<br>
<br>
Fix the race condition introduced with Debugger>>'ErrorRecursion' which resulted in the Emergency Evaluator being opened too eagerly (and unable to be closed!) -- even when there was no recursion.<br>
<br>
=============== Diff against Morphic-eem.1332 ===============<br>
<br>
Item was changed:<br>
----- Method: Debugger class>>morphicOpenContext:<wbr>label:contents: (in category '*Morphic-opening') -----<br>
+ morphicOpenContext: aContext label: aString contents: contentsStringOrNil<br>
- morphicOpenContext: aContext label: aString contents: contentsStringOrNil<br>
"Open a notifier in response to an error, halt, or notify. A notifier view just shows a short view of the sender stack and provides a menu that lets the user open a full debugger."<br>
+ "Simulation guard"<br>
+ <primitive: 19><br>
+ ErrorRecursionGuard critical:<br>
+ [ ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:<br>
+ [ Smalltalk<br>
+ logSqueakError: aString<br>
+ inContext: aContext ].<br>
+ ErrorRecursion ifTrue:<br>
+ [ ErrorRecursion := false.<br>
+ self primitiveError: aString ].<br>
+ ErrorRecursion := true.<br>
+ self<br>
+ informExistingDebugger: aContext<br>
+ label: aString.<br>
+ (Debugger morphicContext: aContext)<br>
+ openNotifierContents: contentsStringOrNil<br>
+ label: aString.<br>
- <primitive: 19> "Simulation guard"<br>
- ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:<br>
- [Smalltalk logSqueakError: aString inContext: aContext].<br>
- ErrorRecursion ifTrue:[<br>
ErrorRecursion := false.<br>
+ Processor activeProcess suspend ]!<br>
- self primitiveError: aString].<br>
- ErrorRecursion := true.<br>
- self informExistingDebugger: aContext label: aString.<br>
- (Debugger morphicContext: aContext)<br>
- openNotifierContents: contentsStringOrNil<br>
- label: aString.<br>
- ErrorRecursion := false.<br>
- Processor activeProcess suspend.!<br>
<br>
Item was changed:<br>
----- Method: Debugger class>>morphicOpenOn:context:<wbr>label:contents:fullView: (in category '*Morphic-opening') -----<br>
+ morphicOpenOn: process context: context label: title contents: contentsStringOrNil fullView: full<br>
- morphicOpenOn: process context: context label: title contents: contentsStringOrNil fullView: full<br>
"Open a notifier in response to an error, halt, or notify. A notifier view just shows a short view of the sender stack and provides a menu that lets the user open a full debugger."<br>
+ ErrorRecursionGuard critical:<br>
+ [ | errorWasInUIProcess debugger |<br>
+ ErrorRecursion ifTrue:<br>
+ [ "self assert: process == Project current uiProcess -- DOCUMENTATION ONLY"<br>
+ ErrorRecursion := false.<br>
+ ^ Project current handleFatalDrawingError: title ].<br>
+ [ ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:<br>
+ [ Smalltalk<br>
+ logSqueakError: title<br>
+ inContext: context ] ]<br>
+ on: Error<br>
+ do: [ : ex | ex return: nil ].<br>
+ ErrorRecursion := true.<br>
+ errorWasInUIProcess := Project current spawnNewProcessIfThisIsUI: process.<br>
+ "Schedule debugging in deferred UI message because<br>
-<br>
- | errorWasInUIProcess debugger |<br>
- ErrorRecursion ifTrue: [<br>
- "self assert: process == Project current uiProcess -- DOCUMENTATION ONLY"<br>
- ErrorRecursion := false.<br>
- ^ Project current handleFatalDrawingError: title].<br>
-<br>
- [ErrorRecursion not & Preferences logDebuggerStackToFile<br>
- ifTrue: [Smalltalk logSqueakError: title inContext: context]] on: Error do: [:ex | ex return: nil].<br>
-<br>
- ErrorRecursion := true.<br>
-<br>
- errorWasInUIProcess := Project current spawnNewProcessIfThisIsUI: process.<br>
-<br>
- "Schedule debugging in deferred UI message because<br>
1) If process is the current UI process, it is already broken.<br>
2) If process is some other process, it must not execute UI code"<br>
+ Project current addDeferredUIMessage:<br>
+ [ debugger := self new<br>
+ process: process<br>
+ controller: nil<br>
+ context: context.<br>
+ full<br>
+ ifTrue: [ debugger openFullNoSuspendLabel: title ]<br>
+ ifFalse:<br>
+ [ debugger<br>
+ openNotifierContents: contentsStringOrNil<br>
+ label: title ].<br>
+ debugger errorWasInUIProcess: errorWasInUIProcess.<br>
+ "Try drawing the debugger tool at least once to avoid freeze."<br>
+ Project current world displayWorldSafely.<br>
+ ErrorRecursion := false ].<br>
+ process suspend ]!<br>
- Project current addDeferredUIMessage: [<br>
- debugger := self new process: process controller: nil context: context.<br>
- full<br>
- ifTrue: [debugger openFullNoSuspendLabel: title]<br>
- ifFalse: [debugger openNotifierContents: contentsStringOrNil label: title].<br>
- debugger errorWasInUIProcess: errorWasInUIProcess.<br>
-<br>
- "Try drawing the debugger tool at least once to avoid freeze."<br>
- Project current world displayWorldSafely.<br>
-<br>
- ErrorRecursion := false.<br>
- ].<br>
- process suspend.<br>
- !<br>
<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="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>