From 692f9c28ebfbb6bb7e42e957d2ebfae340ddca42 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 3 Feb 2022 17:41:46 +0000 Subject: [PATCH] [dds] Ensure thread startup is not handled twice in DAP when attaching Should fix the flakes seen in https://github.com/dart-lang/sdk/issues/48274 caused by Isolate PauseStart events arriving during initialization but being delayed until initialization complete, and then auto-resuming isolates that ere deliberately being left paused. Change-Id: Iee32731b92fe96b6b41e7e78c01af2f5e843ea92 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/231460 Reviewed-by: Ben Konyi Commit-Queue: Ben Konyi --- pkg/dds/lib/src/dap/isolate_manager.dart | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart index ee487945b86..d13e8fad5e8 100644 --- a/pkg/dds/lib/src/dap/isolate_manager.dart +++ b/pkg/dds/lib/src/dap/isolate_manager.dart @@ -445,11 +445,11 @@ class IsolateManager { } else if (eventKind == vm.EventKind.kPauseStart) { // Don't resume from a PauseStart if this has already happened (see // comments on [thread.hasBeenStarted]). - if (!thread.hasBeenStarted) { + if (!thread.startupHandled) { + thread.startupHandled = true; // If requested, automatically resume. Otherwise send a Stopped event to // inform the client UI the thread is paused. if (resumeIfStarting) { - thread.hasBeenStarted = true; await resumeThread(thread.threadId); } else { sendStoppedOnEntryEvent(thread.threadId); @@ -730,7 +730,11 @@ class ThreadInfo { int? exceptionReference; var paused = false; - /// Tracks whether an isolate has been started from its PauseStart state. + /// Tracks whether an isolates startup routine has been handled. + /// + /// The startup routine will either automatically resume the isolate or send + /// a stopped-on-entry event, depending on whether we're launching or + /// attaching. /// /// This is used to prevent trying to resume a thread twice if a PauseStart /// event arrives around the same time that are our initialization code (which @@ -739,7 +743,12 @@ class ThreadInfo { /// /// If we send a duplicate resume, it could trigger an unwanted resume for a /// breakpoint or exception that occur early on. - bool hasBeenStarted = false; + /// + /// In the case of attach, a similar race exists.. The initialization may + /// choose not to resume the isolate (so we can attach to a VM with paused + /// isolates) but then a PauseStart event that arrived during initialization + /// could trigger a resume that we don't want. + bool startupHandled = false; /// The most recent pauseEvent for this isolate. vm.Event? pauseEvent;