The Trunk: Morphic-cmm.1333.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Morphic-cmm.1333.mcz

commits-2
Chris Muller uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-cmm.1333.mcz

==================== Summary ====================

Name: Morphic-cmm.1333
Author: cmm
Time: 4 April 2017, 3:52:04.9926 pm
UUID: 64767f0b-c6c4-4c32-97a9-d122c30222d5
Ancestors: Morphic-eem.1332

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.

=============== Diff against Morphic-eem.1332 ===============

Item was changed:
  ----- Method: Debugger class>>morphicOpenContext:label:contents: (in category '*Morphic-opening') -----
+ morphicOpenContext: aContext label: aString contents: contentsStringOrNil
- morphicOpenContext: aContext label: aString contents: contentsStringOrNil
  "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."
+ "Simulation guard"
+ <primitive: 19>
+ ErrorRecursionGuard critical:
+ [ ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:
+ [ Smalltalk
+ logSqueakError: aString
+ inContext: aContext ].
+ ErrorRecursion ifTrue:
+ [ ErrorRecursion := false.
+ self primitiveError: aString ].
+ ErrorRecursion := true.
+ self
+ informExistingDebugger: aContext
+ label: aString.
+ (Debugger morphicContext: aContext)
+ openNotifierContents: contentsStringOrNil
+ label: aString.
- <primitive: 19> "Simulation guard"
- ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:
- [Smalltalk logSqueakError: aString inContext: aContext].
- ErrorRecursion ifTrue:[
  ErrorRecursion := false.
+ Processor activeProcess suspend ]!
- self primitiveError: aString].
- ErrorRecursion := true.
- self informExistingDebugger: aContext label: aString.
- (Debugger morphicContext: aContext)
- openNotifierContents: contentsStringOrNil
- label: aString.
- ErrorRecursion := false.
- Processor activeProcess suspend.!

Item was changed:
  ----- Method: Debugger class>>morphicOpenOn:context:label:contents:fullView: (in category '*Morphic-opening') -----
+ morphicOpenOn: process context: context label: title contents: contentsStringOrNil fullView: full
- morphicOpenOn: process context: context label: title contents: contentsStringOrNil fullView: full
  "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."
+ ErrorRecursionGuard critical:
+ [ | errorWasInUIProcess debugger |
+ ErrorRecursion ifTrue:
+ [ "self assert: process == Project current uiProcess -- DOCUMENTATION ONLY"
+ ErrorRecursion := false.
+ ^ Project current handleFatalDrawingError: title ].
+ [ ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:
+ [ Smalltalk
+ logSqueakError: title
+ inContext: context ] ]
+ on: Error
+ do: [ : ex | ex return: nil ].
+ ErrorRecursion := true.
+ errorWasInUIProcess := Project current spawnNewProcessIfThisIsUI: process.
+ "Schedule debugging in deferred UI message because
-
- | errorWasInUIProcess debugger |
- ErrorRecursion ifTrue: [
- "self assert: process == Project current uiProcess -- DOCUMENTATION ONLY"
- ErrorRecursion := false.
- ^ Project current handleFatalDrawingError: title].
-
- [ErrorRecursion not & Preferences logDebuggerStackToFile
- ifTrue: [Smalltalk logSqueakError: title inContext: context]] on: Error do: [:ex | ex return: nil].
-
- ErrorRecursion := true.
-
- errorWasInUIProcess := Project current spawnNewProcessIfThisIsUI: process.
-
- "Schedule debugging in deferred UI message because
  1) If process is the current UI process, it is already broken.
  2) If process is some other process, it must not execute UI code"
+ Project current addDeferredUIMessage:
+ [ debugger := self new
+ process: process
+ controller: nil
+ context: context.
+ full
+ ifTrue: [ debugger openFullNoSuspendLabel: title ]
+ ifFalse:
+ [ debugger
+ openNotifierContents: contentsStringOrNil
+ label: title ].
+ debugger errorWasInUIProcess: errorWasInUIProcess.
+ "Try drawing the debugger tool at least once to avoid freeze."
+ Project current world displayWorldSafely.
+ ErrorRecursion := false ].
+ process suspend ]!
- Project current addDeferredUIMessage: [
- debugger := self new process: process controller: nil context: context.
- full
- ifTrue: [debugger openFullNoSuspendLabel: title]
- ifFalse: [debugger openNotifierContents: contentsStringOrNil label: title].
- debugger errorWasInUIProcess: errorWasInUIProcess.
-
- "Try drawing the debugger tool at least once to avoid freeze."
- Project current world displayWorldSafely.
-
- ErrorRecursion := false.
- ].
- process suspend.
- !


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-cmm.1333.mcz

Eliot Miranda-2
Hi Chris,

    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.

One should never do this:

      aMutexOrSemaphore critical:
            [self doStuff.
             Processor activeProcess suspend]

I'm revising these cases to pull the suspend out of the critical block.

On Tue, Apr 4, 2017 at 1:52 PM, <[hidden email]> wrote:
Chris Muller uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-cmm.1333.mcz

==================== Summary ====================

Name: Morphic-cmm.1333
Author: cmm
Time: 4 April 2017, 3:52:04.9926 pm
UUID: 64767f0b-c6c4-4c32-97a9-d122c30222d5
Ancestors: Morphic-eem.1332

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.

=============== Diff against Morphic-eem.1332 ===============

Item was changed:
  ----- Method: Debugger class>>morphicOpenContext:label:contents: (in category '*Morphic-opening') -----
+ morphicOpenContext: aContext label: aString contents: contentsStringOrNil
- morphicOpenContext: aContext label: aString contents: contentsStringOrNil
        "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."
+       "Simulation guard"
+       <primitive: 19>
+       ErrorRecursionGuard critical:
+               [ ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:
+                       [ Smalltalk
+                               logSqueakError: aString
+                               inContext: aContext ].
+               ErrorRecursion ifTrue:
+                       [ ErrorRecursion := false.
+                       self primitiveError: aString ].
+               ErrorRecursion := true.
+               self
+                       informExistingDebugger: aContext
+                       label: aString.
+               (Debugger morphicContext: aContext)
+                       openNotifierContents: contentsStringOrNil
+                       label: aString.
-       <primitive: 19> "Simulation guard"
-       ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:
-               [Smalltalk logSqueakError: aString inContext: aContext].
-       ErrorRecursion ifTrue:[
                ErrorRecursion := false.
+               Processor activeProcess suspend ]!
-               self primitiveError: aString].
-       ErrorRecursion := true.
-       self informExistingDebugger: aContext label: aString.
-       (Debugger morphicContext: aContext)
-               openNotifierContents: contentsStringOrNil
-               label: aString.
-       ErrorRecursion := false.
-       Processor activeProcess suspend.!

Item was changed:
  ----- Method: Debugger class>>morphicOpenOn:context:label:contents:fullView: (in category '*Morphic-opening') -----
+ morphicOpenOn: process context: context label: title contents: contentsStringOrNil fullView: full
- morphicOpenOn: process context: context label: title contents: contentsStringOrNil fullView: full
        "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."
+       ErrorRecursionGuard critical:
+               [ | errorWasInUIProcess debugger |
+               ErrorRecursion ifTrue:
+                       [ "self assert: process == Project current uiProcess -- DOCUMENTATION ONLY"
+                       ErrorRecursion := false.
+                       ^ Project current handleFatalDrawingError: title ].
+               [ ErrorRecursion not & Preferences logDebuggerStackToFile ifTrue:
+                       [ Smalltalk
+                               logSqueakError: title
+                               inContext: context ] ]
+                       on: Error
+                       do: [ : ex | ex return: nil ].
+               ErrorRecursion := true.
+               errorWasInUIProcess := Project current spawnNewProcessIfThisIsUI: process.
+               "Schedule debugging in deferred UI message because
-
-       | errorWasInUIProcess debugger |
-       ErrorRecursion ifTrue: [
-               "self assert: process == Project current uiProcess -- DOCUMENTATION ONLY"
-               ErrorRecursion := false.
-               ^ Project current handleFatalDrawingError: title].
-
-       [ErrorRecursion not & Preferences logDebuggerStackToFile
-               ifTrue: [Smalltalk logSqueakError: title inContext: context]] on: Error do: [:ex | ex return: nil].
-
-       ErrorRecursion := true.
-
-       errorWasInUIProcess := Project current spawnNewProcessIfThisIsUI: process.
-
-       "Schedule debugging in deferred UI message because
                1) If process is the current UI process, it is already broken.
                2) If process is some other process, it must not execute UI code"
+               Project current addDeferredUIMessage:
+                       [ debugger := self new
+                               process: process
+                               controller: nil
+                               context: context.
+                       full
+                               ifTrue: [ debugger openFullNoSuspendLabel: title ]
+                               ifFalse:
+                                       [ debugger
+                                               openNotifierContents: contentsStringOrNil
+                                               label: title ].
+                       debugger errorWasInUIProcess: errorWasInUIProcess.
+                       "Try drawing the debugger tool at least once to avoid freeze."
+                       Project current world displayWorldSafely.
+                       ErrorRecursion := false ].
+               process suspend ]!
-       Project current addDeferredUIMessage: [
-               debugger := self new process: process controller: nil context: context.
-               full
-                       ifTrue: [debugger openFullNoSuspendLabel: title]
-                       ifFalse: [debugger openNotifierContents: contentsStringOrNil label: title].
-               debugger errorWasInUIProcess: errorWasInUIProcess.
-
-               "Try drawing the debugger tool at least once to avoid freeze."
-               Project current world displayWorldSafely.
-
-               ErrorRecursion := false.
-       ].
-       process suspend.
- !





--
_,,,^..^,,,_
best, Eliot