From 8191d2033d85575a25e38475aa421211ffb0e7be Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Mon, 5 Jun 2023 19:37:11 -0300 Subject: [PATCH 1/2] We were sending the scriptId of a context that was already destroyed and vscode-js-debug extension started to consider this information that was ignored before. --- .../Firefox/FirefoxMonoProxy.cs | 8 ++-- .../debugger/BrowserDebugProxy/MonoProxy.cs | 39 ++++++++++++++----- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs index 25c7dc8a3a9d04..8b446d643cdda5 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs @@ -23,7 +23,7 @@ public FirefoxMonoProxy(ILogger logger, string loggerId = null, ProxyOptions opt public FirefoxExecutionContext GetContextFixefox(SessionId sessionId) { - if (contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return context as FirefoxExecutionContext; throw new ArgumentException($"Invalid Session: \"{sessionId}\"", nameof(sessionId)); } @@ -316,7 +316,7 @@ protected override async Task AcceptCommand(MessageId sessionId, JObject a { case "resume": { - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return false; context.PausedOnWasm = false; if (context.CallStack == null) @@ -380,7 +380,7 @@ protected override async Task AcceptCommand(MessageId sessionId, JObject a } case "setBreakpoint": { - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return false; var req = JObject.FromObject(new { @@ -420,7 +420,7 @@ protected override async Task AcceptCommand(MessageId sessionId, JObject a } case "removeBreakpoint": { - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return false; Result resp = await SendCommand(sessionId, "", args, token); diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 11cc46f7978f7c..ec3d7b02c00e61 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -24,7 +24,7 @@ internal class MonoProxy : DevToolsProxy internal string CachePathSymbolServer { get; private set; } private readonly HashSet sessions = new HashSet(); private static readonly string[] s_executionContextIndependentCDPCommandNames = { "DotnetDebugger.setDebuggerProperty", "DotnetDebugger.runTests" }; - protected Dictionary contexts = new Dictionary(); + protected Dictionary> contexts = new Dictionary>(); public static HttpClient HttpClient => new HttpClient(); @@ -45,7 +45,7 @@ public MonoProxy(ILogger logger, int runtimeId = 0, string loggerId = "", ProxyO internal ExecutionContext GetContext(SessionId sessionId) { - if (contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return context; throw new ArgumentException($"Invalid Session: \"{sessionId}\"", nameof(sessionId)); @@ -53,16 +53,27 @@ internal ExecutionContext GetContext(SessionId sessionId) private bool UpdateContext(SessionId sessionId, ExecutionContext executionContext, out ExecutionContext previousExecutionContext) { - bool previous = contexts.TryGetValue(sessionId, out previousExecutionContext); - contexts[sessionId] = executionContext; + bool previous = TryGetExecutionContextValue(sessionId, out previousExecutionContext); + if (!previous) + contexts[sessionId] = new(); + contexts[sessionId].Add(executionContext); return previous; } + internal bool TryGetExecutionContextValue(SessionId id, out ExecutionContext executionContext) + { + executionContext = null; + if (!contexts.TryGetValue(id, out List contextList)) + return false; + executionContext = contextList.Last(); + return true; + } + internal virtual Task SendMonoCommand(SessionId id, MonoCommands cmd, CancellationToken token) => SendCommand(id, "Runtime.evaluate", JObject.FromObject(cmd), token); internal void SendLog(SessionId sessionId, string message, CancellationToken token, string type = "warning") { - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return; /*var o = JObject.FromObject(new { @@ -151,6 +162,13 @@ protected override async Task AcceptEvent(SessionId sessionId, JObject par } return true; } + case "Runtime.executionContextDestroyed": + { + int id = args["executionContextId"].Value(); + contexts.TryGetValue(sessionId, out var contextList); + contextList.RemoveAll(x => x.Id == id); + return false; + } case "Debugger.paused": { @@ -188,7 +206,7 @@ protected async Task OnDebuggerPaused(SessionId sessionId, JObject args, C if (args["asyncStackTraceId"] != null) { - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) return false; if (context.CopyDataFromParentContext()) { @@ -254,7 +272,7 @@ protected async Task OnDebuggerPaused(SessionId sessionId, JObject args, C protected void CreateWorkerExecutionContext(SessionId workerSessionId, SessionId originSessionId) { - if (!contexts.TryGetValue(originSessionId, out ExecutionContext context)) + if (!TryGetExecutionContextValue(originSessionId, out ExecutionContext context)) { logger.LogDebug($"Origin sessionId does not exist - {originSessionId}"); return; @@ -264,7 +282,8 @@ protected void CreateWorkerExecutionContext(SessionId workerSessionId, SessionId logger.LogDebug($"Worker sessionId already exists - {originSessionId}"); return; } - contexts[workerSessionId] = context.CreateChildAsyncExecutionContext(workerSessionId); + contexts[workerSessionId] = new(); + contexts[workerSessionId].Add(context.CreateChildAsyncExecutionContext(workerSessionId)); } protected virtual async Task SendResume(SessionId id, CancellationToken token) @@ -273,7 +292,7 @@ protected virtual async Task SendResume(SessionId id, CancellationToken token) } protected async Task IsRuntimeAlreadyReadyAlready(SessionId sessionId, CancellationToken token) { - if (contexts.TryGetValue(sessionId, out ExecutionContext context) && context.IsRuntimeReady) + if (TryGetExecutionContextValue(sessionId, out ExecutionContext context) && context.IsRuntimeReady) return true; Result res = await SendMonoCommand(sessionId, MonoCommands.IsRuntimeReady(RuntimeId), token); @@ -298,7 +317,7 @@ protected override async Task AcceptCommand(MessageId id, JObject parms, C if (id == SessionId.Null) await AttachToTarget(id, token); - if (!contexts.TryGetValue(id, out ExecutionContext context) && !s_executionContextIndependentCDPCommandNames.Contains(method)) + if (!TryGetExecutionContextValue(id, out ExecutionContext context) && !s_executionContextIndependentCDPCommandNames.Contains(method)) { if (method == "Debugger.setPauseOnExceptions") { From f3a6378ee8c80d585f47f21203fa4b7a366554fe Mon Sep 17 00:00:00 2001 From: "Thays Tagliaferri de Grazia (SHE/HER)" Date: Wed, 7 Jun 2023 19:32:10 -0300 Subject: [PATCH 2/2] addressing @radical comments --- .../Firefox/FirefoxMonoProxy.cs | 8 +++---- .../debugger/BrowserDebugProxy/MonoProxy.cs | 23 +++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs index 8b446d643cdda5..8c8605cf25f726 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs @@ -23,7 +23,7 @@ public FirefoxMonoProxy(ILogger logger, string loggerId = null, ProxyOptions opt public FirefoxExecutionContext GetContextFixefox(SessionId sessionId) { - if (TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return context as FirefoxExecutionContext; throw new ArgumentException($"Invalid Session: \"{sessionId}\"", nameof(sessionId)); } @@ -316,7 +316,7 @@ protected override async Task AcceptCommand(MessageId sessionId, JObject a { case "resume": { - if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (!TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return false; context.PausedOnWasm = false; if (context.CallStack == null) @@ -380,7 +380,7 @@ protected override async Task AcceptCommand(MessageId sessionId, JObject a } case "setBreakpoint": { - if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (!TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return false; var req = JObject.FromObject(new { @@ -420,7 +420,7 @@ protected override async Task AcceptCommand(MessageId sessionId, JObject a } case "removeBreakpoint": { - if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (!TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return false; Result resp = await SendCommand(sessionId, "", args, token); diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index ec3d7b02c00e61..ed2495c469cf35 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -45,7 +45,7 @@ public MonoProxy(ILogger logger, int runtimeId = 0, string loggerId = "", ProxyO internal ExecutionContext GetContext(SessionId sessionId) { - if (TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return context; throw new ArgumentException($"Invalid Session: \"{sessionId}\"", nameof(sessionId)); @@ -53,18 +53,20 @@ internal ExecutionContext GetContext(SessionId sessionId) private bool UpdateContext(SessionId sessionId, ExecutionContext executionContext, out ExecutionContext previousExecutionContext) { - bool previous = TryGetExecutionContextValue(sessionId, out previousExecutionContext); + bool previous = TryGetCurrentExecutionContextValue(sessionId, out previousExecutionContext); if (!previous) contexts[sessionId] = new(); contexts[sessionId].Add(executionContext); return previous; } - internal bool TryGetExecutionContextValue(SessionId id, out ExecutionContext executionContext) + internal bool TryGetCurrentExecutionContextValue(SessionId id, out ExecutionContext executionContext) { executionContext = null; if (!contexts.TryGetValue(id, out List contextList)) return false; + if (contextList.Count == 0) + return false; executionContext = contextList.Last(); return true; } @@ -73,7 +75,7 @@ internal bool TryGetExecutionContextValue(SessionId id, out ExecutionContext exe internal void SendLog(SessionId sessionId, string message, CancellationToken token, string type = "warning") { - if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (!TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return; /*var o = JObject.FromObject(new { @@ -165,8 +167,11 @@ protected override async Task AcceptEvent(SessionId sessionId, JObject par case "Runtime.executionContextDestroyed": { int id = args["executionContextId"].Value(); - contexts.TryGetValue(sessionId, out var contextList); + if (!contexts.TryGetValue(sessionId, out var contextList)) + return false; contextList.RemoveAll(x => x.Id == id); + if (contextList.Count == 0) + contexts.Remove(sessionId); return false; } @@ -206,7 +211,7 @@ protected async Task OnDebuggerPaused(SessionId sessionId, JObject args, C if (args["asyncStackTraceId"] != null) { - if (!TryGetExecutionContextValue(sessionId, out ExecutionContext context)) + if (!TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context)) return false; if (context.CopyDataFromParentContext()) { @@ -272,7 +277,7 @@ protected async Task OnDebuggerPaused(SessionId sessionId, JObject args, C protected void CreateWorkerExecutionContext(SessionId workerSessionId, SessionId originSessionId) { - if (!TryGetExecutionContextValue(originSessionId, out ExecutionContext context)) + if (!TryGetCurrentExecutionContextValue(originSessionId, out ExecutionContext context)) { logger.LogDebug($"Origin sessionId does not exist - {originSessionId}"); return; @@ -292,7 +297,7 @@ protected virtual async Task SendResume(SessionId id, CancellationToken token) } protected async Task IsRuntimeAlreadyReadyAlready(SessionId sessionId, CancellationToken token) { - if (TryGetExecutionContextValue(sessionId, out ExecutionContext context) && context.IsRuntimeReady) + if (TryGetCurrentExecutionContextValue(sessionId, out ExecutionContext context) && context.IsRuntimeReady) return true; Result res = await SendMonoCommand(sessionId, MonoCommands.IsRuntimeReady(RuntimeId), token); @@ -317,7 +322,7 @@ protected override async Task AcceptCommand(MessageId id, JObject parms, C if (id == SessionId.Null) await AttachToTarget(id, token); - if (!TryGetExecutionContextValue(id, out ExecutionContext context) && !s_executionContextIndependentCDPCommandNames.Contains(method)) + if (!TryGetCurrentExecutionContextValue(id, out ExecutionContext context) && !s_executionContextIndependentCDPCommandNames.Contains(method)) { if (method == "Debugger.setPauseOnExceptions") {