From 1bd0c119813637243d203cd0a34bd16ba1653c91 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Fri, 26 Jun 2026 12:52:05 -0700 Subject: [PATCH 1/4] Fix UI-thread hang when resuming the target during launch AD7Engine.ContinueFromSynchronousEvent (the AD7ProgramCreateEvent path) dispatched ResumeFromLaunch through the blocking WorkerThread.RunOperation, coupling the VS UI/SDM thread to debugger I/O. Over a slow or remote (SSH) transport, AD7Engine.ContinueFromSynchronousEvent + ResumeFromLaunch could stall for a long time or indefinitely, producing a Watson hang. - Add WorkerThread.PostAsyncOperation, a non-blocking dispatch that claims the running-op slot (so later AD7 operations still serialize behind it) but returns without waiting for completion. Faults from the operation are routed to an onError callback from both the synchronous and async completion paths. - ContinueFromSynchronousEvent now posts ResumeFromLaunch via PostAsyncOperation and returns S_OK immediately, so the UI thread is no longer blocked. Resume faults report through SendStartDebuggingError + Terminate via onError, and synchronous failures (OnLoadComplete or the dispatch itself) are still handled locally, since the SDM drops errors returned from this method. - [x] Built and tested locally - [x] Debugged through WSL Linux gdb and the async resume completed. --- src/MIDebugEngine/AD7.Impl/AD7Engine.cs | 28 +++--- .../Engine.Impl/OperationThread.cs | 96 +++++++++++++++++++ 2 files changed, 108 insertions(+), 16 deletions(-) diff --git a/src/MIDebugEngine/AD7.Impl/AD7Engine.cs b/src/MIDebugEngine/AD7.Impl/AD7Engine.cs index ffb8146b8..65dcf5681 100755 --- a/src/MIDebugEngine/AD7.Impl/AD7Engine.cs +++ b/src/MIDebugEngine/AD7.Impl/AD7Engine.cs @@ -319,28 +319,24 @@ public int ContinueFromSynchronousEvent(IDebugEvent2 eventObject) { if (eventObject is AD7ProgramCreateEvent) { - Exception exception = null; - try { _engineCallback.OnLoadComplete(); - // At this point breakpoints and exception settings have been sent down, so we can resume the target - _pollThread.RunOperation(() => - { - return _debuggedProcess.ResumeFromLaunch(); - }); + + // Resume the target on the worker thread without blocking the UI thread this runs on. + // Resume faults are reported via onError, since the SDM drops errors returned from here. + _pollThread.PostAsyncOperation( + () => _debuggedProcess.ResumeFromLaunch(), + (exception) => + { + SendStartDebuggingError(exception); + _debuggedProcess.Terminate(); + }); } catch (Exception e) { - exception = e; - // Return from the catch block so that we can let the exception unwind - the stack can get kind of big - } - - if (exception != null) - { - // If something goes wrong, report the error and then stop debugging. The SDM will drop errors - // from ContinueFromSynchronousEvent, so we want to deal with them ourself. - SendStartDebuggingError(exception); + // Report synchronous failures ourselves, since the SDM drops errors returned from here. + SendStartDebuggingError(e); _debuggedProcess.Terminate(); } diff --git a/src/MIDebugEngine/Engine.Impl/OperationThread.cs b/src/MIDebugEngine/Engine.Impl/OperationThread.cs index 2c5d71a24..b59843c20 100755 --- a/src/MIDebugEngine/Engine.Impl/OperationThread.cs +++ b/src/MIDebugEngine/Engine.Impl/OperationThread.cs @@ -37,6 +37,12 @@ private class OperationDescriptor /// Delegate that was added via 'RunOperation'. Is of type 'Operation' or 'AsyncOperation' /// public readonly Delegate Target; + + /// + /// Handler invoked on the worker thread if the operation faults. Only set for PostAsyncOperation. + /// + public Action ErrorHandler; + public ExceptionDispatchInfo ExceptionDispatchInfo; public Task Task; private bool _isStarted; @@ -121,6 +127,20 @@ public void RunOperation(string text, CancellationTokenSource canTokenSource, As SetOperationInternalWithProgress(op, text, canTokenSource); } + /// + /// Send an async operation to the worker thread, claiming the running-op slot but returning without + /// waiting for it to complete. Later operations still serialize behind it. Faults are reported to + /// . + /// + public void PostAsyncOperation(AsyncOperation op, Action onError) + { + if (op == null) + throw new ArgumentNullException(nameof(op)); + if (onError == null) + throw new ArgumentNullException(nameof(onError)); + + PostAsyncOperationInternal(op, onError); + } public void Close() { @@ -189,6 +209,27 @@ internal void SetOperationInternalWithProgress(AsyncProgressOperation op, string } } } + + internal void PostAsyncOperationInternal(AsyncOperation op, Action onError) + { + // If this is called on the Worker thread it will deadlock + Debug.Assert(!IsPollThread()); + + while (true) + { + if (_isClosed) + throw new ObjectDisposedException("WorkerThread"); + + // Wait for the slot so this serializes behind any in-flight operation. + _runningOpCompleteEvent.WaitOne(); + + if (TryPostAsyncOperationInternal(op, onError)) + { + return; + } + } + } + public void PostOperation(Operation op) { if (op == null) @@ -276,7 +317,28 @@ private bool TrySetOperationInternalWithProgress(AsyncProgressOperation op, stri return false; } + private bool TryPostAsyncOperationInternal(AsyncOperation op, Action onError) + { + lock (_eventLock) + { + if (_isClosed) + throw new ObjectDisposedException("WorkerThread"); + + if (_runningOp == null) + { + _runningOpCompleteEvent.Reset(); + + _runningOp = new OperationDescriptor(op) { ErrorHandler = onError }; + + _opSet.Set(); + // Unlike TrySetOperationInternal, do not wait for completion; faults are routed to onError. + return true; + } + } + + return false; + } // Thread routine for the poll loop. It handles calls coming in from the debug engine as well as polling for debug events. private void ThreadFunc() @@ -333,11 +395,20 @@ private void ThreadFunc() if (!completeAsync) { + // Capture the fault before clearing the slot so a synchronous throw is still reported. + Action errorHandler = runningOp.ErrorHandler; + ExceptionDispatchInfo exceptionDispatchInfo = runningOp.ExceptionDispatchInfo; + runningOp.MarkComplete(); Debug.Assert(_runningOp == runningOp, "How did m_runningOp change?"); _runningOp = null; _runningOpCompleteEvent.Set(); + + if (errorHandler != null && exceptionDispatchInfo != null) + { + InvokeErrorHandler(errorHandler, exceptionDispatchInfo.SourceException); + } } } @@ -389,8 +460,33 @@ internal void OnAsyncRunningOpComplete(Task t) } } _runningOp.MarkComplete(); + + // Capture the fault before clearing the slot so it is routed to the handler, not discarded. + Action errorHandler = _runningOp.ErrorHandler; + ExceptionDispatchInfo exceptionDispatchInfo = _runningOp.ExceptionDispatchInfo; + _runningOp = null; _runningOpCompleteEvent.Set(); + + if (errorHandler != null && exceptionDispatchInfo != null) + { + InvokeErrorHandler(errorHandler, exceptionDispatchInfo.SourceException); + } + } + + private void InvokeErrorHandler(Action errorHandler, Exception exception) + { + try + { + errorHandler(exception); + } + catch (Exception e) when (ExceptionHelper.BeforeCatch(e, Logger, reportOnlyCorrupting: false)) + { + if (PostedOperationErrorEvent != null) + { + PostedOperationErrorEvent(this, e); + } + } } internal bool IsPollThread() From 42e8354bc231f222a9e9a55212393bb34656db3c Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Tue, 14 Jul 2026 15:49:46 -0700 Subject: [PATCH 2/4] Serialize PostAsyncOperation fault handler and document its threading Address PR review: document that the ErrorHandler for an async operation may be invoked on a non-worker (task completion) thread, and invoke it while the running-op slot is still held so it does not run concurrently with the next queued operation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b5e2926-4bce-44b8-bfab-8b9b7f36a615 --- src/MIDebugEngine/Engine.Impl/OperationThread.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/MIDebugEngine/Engine.Impl/OperationThread.cs b/src/MIDebugEngine/Engine.Impl/OperationThread.cs index b59843c20..b8f283104 100755 --- a/src/MIDebugEngine/Engine.Impl/OperationThread.cs +++ b/src/MIDebugEngine/Engine.Impl/OperationThread.cs @@ -39,7 +39,8 @@ private class OperationDescriptor public readonly Delegate Target; /// - /// Handler invoked on the worker thread if the operation faults. Only set for PostAsyncOperation. + /// Handler invoked if the operation faults. For async operations, this may be invoked on a non-worker thread. + /// Only set for PostAsyncOperation. /// public Action ErrorHandler; @@ -465,13 +466,15 @@ internal void OnAsyncRunningOpComplete(Task t) Action errorHandler = _runningOp.ErrorHandler; ExceptionDispatchInfo exceptionDispatchInfo = _runningOp.ExceptionDispatchInfo; - _runningOp = null; - _runningOpCompleteEvent.Set(); - + // Invoke the handler while the running-op slot is still held so it does not run concurrently + // with the next queued operation. This may still run on the task's completion thread. if (errorHandler != null && exceptionDispatchInfo != null) { InvokeErrorHandler(errorHandler, exceptionDispatchInfo.SourceException); } + + _runningOp = null; + _runningOpCompleteEvent.Set(); } private void InvokeErrorHandler(Action errorHandler, Exception exception) From d575ea9a168f993dd70a4ffef9a389f60bee751c Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 15 Jul 2026 14:44:36 -0700 Subject: [PATCH 3/4] Address PR comments --- .../Engine.Impl/OperationThread.cs | 83 +++++++++++-------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/src/MIDebugEngine/Engine.Impl/OperationThread.cs b/src/MIDebugEngine/Engine.Impl/OperationThread.cs index b8f283104..d7af77a92 100755 --- a/src/MIDebugEngine/Engine.Impl/OperationThread.cs +++ b/src/MIDebugEngine/Engine.Impl/OperationThread.cs @@ -28,6 +28,7 @@ internal class WorkerThread : IDisposable private readonly ManualResetEvent _runningOpCompleteEvent; // fired when either m_syncOp finishes, or the kick off of m_async private readonly Object _eventLock = new object(); // Locking on an event directly can cause Mono to stop responding. private readonly Queue _postedOperations; // queue of fire-and-forget operations + private readonly Queue<(AsyncOperation Operation, Action OnError)> _postedAsyncOperations; // queue of fire-and-forget async operations public event EventHandler PostedOperationErrorEvent; @@ -88,6 +89,7 @@ public WorkerThread(Logger logger) _opSet = new AutoResetEvent(false); _runningOpCompleteEvent = new ManualResetEvent(true); _postedOperations = new Queue(); + _postedAsyncOperations = new Queue<(AsyncOperation Operation, Action OnError)>(); _thread = new Thread(new ThreadStart(ThreadFunc)); _thread.Name = "MIDebugger.PollThread"; @@ -129,9 +131,9 @@ public void RunOperation(string text, CancellationTokenSource canTokenSource, As } /// - /// Send an async operation to the worker thread, claiming the running-op slot but returning without - /// waiting for it to complete. Later operations still serialize behind it. Faults are reported to - /// . + /// Queue an async operation to run on the worker thread and return immediately, without waiting for the + /// operation to start or finish. Posted async operations run one at a time, in the order posted, and + /// serialize behind any in-flight operation. Faults are reported to . /// public void PostAsyncOperation(AsyncOperation op, Action onError) { @@ -140,7 +142,17 @@ public void PostAsyncOperation(AsyncOperation op, Action onError) if (onError == null) throw new ArgumentNullException(nameof(onError)); - PostAsyncOperationInternal(op, onError); + if (_isClosed) + throw new ObjectDisposedException("WorkerThread"); + + lock (_postedAsyncOperations) + { + if (_isClosed) + throw new ObjectDisposedException("WorkerThread"); + + _postedAsyncOperations.Enqueue((op, onError)); + _opSet.Set(); + } } public void Close() @@ -211,26 +223,6 @@ internal void SetOperationInternalWithProgress(AsyncProgressOperation op, string } } - internal void PostAsyncOperationInternal(AsyncOperation op, Action onError) - { - // If this is called on the Worker thread it will deadlock - Debug.Assert(!IsPollThread()); - - while (true) - { - if (_isClosed) - throw new ObjectDisposedException("WorkerThread"); - - // Wait for the slot so this serializes behind any in-flight operation. - _runningOpCompleteEvent.WaitOne(); - - if (TryPostAsyncOperationInternal(op, onError)) - { - return; - } - } - } - public void PostOperation(Operation op) { if (op == null) @@ -318,27 +310,33 @@ private bool TrySetOperationInternalWithProgress(AsyncProgressOperation op, stri return false; } - private bool TryPostAsyncOperationInternal(AsyncOperation op, Action onError) + // Called on the poll thread to promote the next posted async operation into the running-op slot when it + // is free. Returns true if an operation was moved into the slot. + private bool TryStartPostedAsyncOperation() { + Debug.Assert(IsPollThread(), "TryStartPostedAsyncOperation must run on the poll thread."); + lock (_eventLock) { - if (_isClosed) - throw new ObjectDisposedException("WorkerThread"); + if (_isClosed || _runningOp != null) + return false; - if (_runningOp == null) + (AsyncOperation Operation, Action OnError) posted; + lock (_postedAsyncOperations) { - _runningOpCompleteEvent.Reset(); + if (_postedAsyncOperations.Count == 0) + return false; - _runningOp = new OperationDescriptor(op) { ErrorHandler = onError }; + posted = _postedAsyncOperations.Dequeue(); + } - _opSet.Set(); + _runningOpCompleteEvent.Reset(); - // Unlike TrySetOperationInternal, do not wait for completion; faults are routed to onError. - return true; - } - } + // Unlike TrySetOperationInternal, no one waits for completion; faults are routed to the handler. + _runningOp = new OperationDescriptor(posted.Operation) { ErrorHandler = posted.OnError }; - return false; + return true; + } } // Thread routine for the poll loop. It handles calls coming in from the debug engine as well as polling for debug events. @@ -355,6 +353,11 @@ private void ThreadFunc() { ranOperation = false; + if (_runningOp == null) + { + TryStartPostedAsyncOperation(); + } + OperationDescriptor runningOp = _runningOp; if (runningOp != null && !runningOp.IsStarted) { @@ -475,6 +478,14 @@ internal void OnAsyncRunningOpComplete(Task t) _runningOp = null; _runningOpCompleteEvent.Set(); + + lock (_postedAsyncOperations) + { + if (_postedAsyncOperations.Count > 0) + { + _opSet.Set(); + } + } } private void InvokeErrorHandler(Action errorHandler, Exception exception) From 75a72d6048c4fad2ff9c00ba43c84dc4fb879750 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 15 Jul 2026 22:20:48 -0700 Subject: [PATCH 4/4] More PR issues --- .../Engine.Impl/OperationThread.cs | 142 +++++++++++++----- 1 file changed, 107 insertions(+), 35 deletions(-) diff --git a/src/MIDebugEngine/Engine.Impl/OperationThread.cs b/src/MIDebugEngine/Engine.Impl/OperationThread.cs index d7af77a92..706f0d626 100755 --- a/src/MIDebugEngine/Engine.Impl/OperationThread.cs +++ b/src/MIDebugEngine/Engine.Impl/OperationThread.cs @@ -183,6 +183,22 @@ public void Close() } } } + + // Fail any queued async operations that will never run now that we are closed, instead of + // dropping them silently. The poll thread won't promote them once _isClosed is set. + while (true) + { + Action onError; + lock (_postedAsyncOperations) + { + if (_postedAsyncOperations.Count == 0) + break; + + onError = _postedAsyncOperations.Dequeue().OnError; + } + + InvokeErrorHandler(onError, new ObjectDisposedException("WorkerThread")); + } } internal void SetOperationInternal(Delegate op) @@ -246,68 +262,98 @@ public void PostOperation(Operation op) private bool TrySetOperationInternal(Delegate op) { - lock (_eventLock) + bool claimed = false; + try { - if (_isClosed) - throw new ObjectDisposedException("WorkerThread"); - - if (_runningOp == null) + lock (_eventLock) { - _runningOpCompleteEvent.Reset(); + if (_isClosed) + throw new ObjectDisposedException("WorkerThread"); - OperationDescriptor runningOp = new OperationDescriptor(op); - _runningOp = runningOp; + if (_runningOp == null) + { + _runningOpCompleteEvent.Reset(); - _opSet.Set(); + OperationDescriptor runningOp = new OperationDescriptor(op); + _runningOp = runningOp; - _runningOpCompleteEvent.WaitOne(); + _opSet.Set(); - Debug.Assert(runningOp.IsComplete, "Why isn't the running op complete?"); + _runningOpCompleteEvent.WaitOne(); + claimed = true; - if (runningOp.ExceptionDispatchInfo != null) - { - runningOp.ExceptionDispatchInfo.Throw(); + Debug.Assert(runningOp.IsComplete, "Why isn't the running op complete?"); + + if (runningOp.ExceptionDispatchInfo != null) + { + runningOp.ExceptionDispatchInfo.Throw(); + } + + return true; } + } - return true; + return false; + } + finally + { + // The running-op slot was just freed and _eventLock is now released. If promotion on the poll + // thread lost the TryEnter race against this method, re-arm _opSet so a queued async operation + // is promoted immediately instead of stranded until the next unrelated wakeup. + if (claimed && HasPostedAsyncOperation()) + { + _opSet.Set(); } } - - return false; } private bool TrySetOperationInternalWithProgress(AsyncProgressOperation op, string text, CancellationTokenSource canTokenSource) { var waitLoop = new HostWaitLoop(text); - lock (_eventLock) + bool claimed = false; + try { - if (_isClosed) - throw new ObjectDisposedException("WorkerThread"); - - if (_runningOp == null) + lock (_eventLock) { - _runningOpCompleteEvent.Reset(); + if (_isClosed) + throw new ObjectDisposedException("WorkerThread"); + + if (_runningOp == null) + { + _runningOpCompleteEvent.Reset(); - OperationDescriptor runningOp = new OperationDescriptor(new AsyncOperation(() => { return op(waitLoop); })); - _runningOp = runningOp; + OperationDescriptor runningOp = new OperationDescriptor(new AsyncOperation(() => { return op(waitLoop); })); + _runningOp = runningOp; - _opSet.Set(); + _opSet.Set(); - waitLoop.Wait(_runningOpCompleteEvent, canTokenSource); + waitLoop.Wait(_runningOpCompleteEvent, canTokenSource); + claimed = true; - Debug.Assert(runningOp.IsComplete, "Why isn't the running op complete?"); + Debug.Assert(runningOp.IsComplete, "Why isn't the running op complete?"); - if (runningOp.ExceptionDispatchInfo != null) - { - runningOp.ExceptionDispatchInfo.Throw(); + if (runningOp.ExceptionDispatchInfo != null) + { + runningOp.ExceptionDispatchInfo.Throw(); + } + + return true; } + } - return true; + return false; + } + finally + { + // The running-op slot was just freed and _eventLock is now released. If promotion on the poll + // thread lost the TryEnter race against this method, re-arm _opSet so a queued async operation + // is promoted immediately instead of stranded until the next unrelated wakeup. + if (claimed && HasPostedAsyncOperation()) + { + _opSet.Set(); } } - - return false; } // Called on the poll thread to promote the next posted async operation into the running-op slot when it @@ -316,7 +362,21 @@ private bool TryStartPostedAsyncOperation() { Debug.Assert(IsPollThread(), "TryStartPostedAsyncOperation must run on the poll thread."); - lock (_eventLock) + // Cheap early-out so the poll loop does not contend on _eventLock when there is nothing to promote. + lock (_postedAsyncOperations) + { + if (_isClosed || _postedAsyncOperations.Count == 0) + return false; + } + + // Never block on _eventLock here: a client in TrySetOperationInternal holds it across + // _runningOpCompleteEvent.WaitOne() until its operation completes, and only the poll thread can + // complete that operation, so blocking here would deadlock. If a client is mid-set, skip promotion; + // it will be retried on a later poll-loop iteration or wakeup. + if (!Monitor.TryEnter(_eventLock)) + return false; + + try { if (_isClosed || _runningOp != null) return false; @@ -337,6 +397,18 @@ private bool TryStartPostedAsyncOperation() return true; } + finally + { + Monitor.Exit(_eventLock); + } + } + + private bool HasPostedAsyncOperation() + { + lock (_postedAsyncOperations) + { + return _postedAsyncOperations.Count > 0; + } } // Thread routine for the poll loop. It handles calls coming in from the debug engine as well as polling for debug events.