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, <commits at source.squeak.org> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20170411/162803c0/attachment.html>