Skip to content

Commit

Permalink
[wasm][debugger][tests] Fix crash in EvaluateOnCallFrameTests on CI (
Browse files Browse the repository at this point in the history
…#69073)

* Clean up debug messages

* [wasm][debugger][tests] Allow MonoProxy instances to get gc'ed

In `TestHarnessProxy`, we recently started adding proxy instances to a
static table, to enable getting the correct exit state from the client.
But the instances were not removed from the table, which prevented the
GC from collecting them.

This manifests as the test process crashing on systems with 8GB of
memory, when running `EvaluateOnCallFrameTests`. It would end up getting
to a resident size of 2.2-2.7G, and then crash after about 30 tests. With these changes, the
proxy gets collected, and the resident memory hovers around 500M.

Future work: this will change once we start using the upcoming app host
work, and thus moving the proxy to a separate process via `dotnet run
--debug`.
  • Loading branch information
radical authored May 10, 2022
1 parent 1368c74 commit f730657
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 48 deletions.
6 changes: 1 addition & 5 deletions src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2256,10 +2256,7 @@ public async Task<GetMembersResult> GetValuesFromDebuggerProxyAttribute(int obje
{
int methodId = await FindDebuggerProxyConstructorIdFor(typeId, token);
if (methodId == -1)
{
logger.LogInformation($"GetValuesFromDebuggerProxyAttribute: could not find proxy constructor id for objectId {objectId}");
return null;
}

using var ctorArgsWriter = new MonoBinaryWriter();
ctorArgsWriter.Write((byte)ValueTypeId.Null);
Expand All @@ -2284,7 +2281,6 @@ public async Task<GetMembersResult> GetValuesFromDebuggerProxyAttribute(int obje
}

var retMethod = await InvokeMethod(ctorArgsWriter.GetParameterBuffer(), methodId, token);
logger.LogInformation($"* GetValuesFromDebuggerProxyAttribute got from InvokeMethod: {retMethod}");
if (!DotnetObjectId.TryParse(retMethod?["value"]?["objectId"]?.Value<string>(), out DotnetObjectId dotnetObjectId))
throw new Exception($"Invoking .ctor ({methodId}) for DebuggerTypeProxy on type {typeId} returned {retMethod}");

Expand All @@ -2296,7 +2292,7 @@ public async Task<GetMembersResult> GetValuesFromDebuggerProxyAttribute(int obje
}
catch (Exception e)
{
logger.LogInformation($"Could not evaluate DebuggerTypeProxyAttribute of type {await GetTypeName(typeId, token)} - {e}");
logger.LogDebug($"Could not evaluate DebuggerTypeProxyAttribute of type {await GetTypeName(typeId, token)} - {e}");
}

return null;
Expand Down
75 changes: 32 additions & 43 deletions src/mono/wasm/debugger/DebuggerTestSuite/TestHarnessProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@ public class TestHarnessProxy

public static readonly Uri Endpoint = new Uri("http://localhost:9400");

// FIXME: use concurrentdictionary?
// And remove the "used" proxy entries
private static readonly ConcurrentBag<(string id, DebuggerProxyBase proxy)> s_proxyTable = new();
private static readonly ConcurrentBag<(string id, Action<RunLoopExitState> handler)> s_exitHandlers = new();
private static readonly ConcurrentBag<(string id, RunLoopExitState state)> s_statusTable = new();
private static readonly ConcurrentDictionary<string, WeakReference<DebuggerProxyBase>> s_proxyTable = new();
private static readonly ConcurrentDictionary<int, WeakReference<Action<RunLoopExitState>>> s_exitHandlers = new();
private static readonly ConcurrentDictionary<string, RunLoopExitState> s_statusTable = new();

public static Task Start(string appPath, string pagePath, string url)
{
Expand Down Expand Up @@ -87,71 +85,62 @@ public static Task Start(string appPath, string pagePath, string url)

public static void RegisterNewProxy(string id, DebuggerProxyBase proxy)
{
if (s_proxyTable.Where(t => t.id == id).Any())
if (s_proxyTable.ContainsKey(id))
throw new ArgumentException($"Proxy with id {id} already exists");

s_proxyTable.Add((id, proxy));
}

private static bool TryGetProxyById(string id, [NotNullWhen(true)] out DebuggerProxyBase? proxy)
{
proxy = null;
IEnumerable<(string id, DebuggerProxyBase proxy)> found = s_proxyTable.Where(t => t.id == id);
if (found.Any())
proxy = found.First().proxy;

return proxy != null;
s_proxyTable[id] = new WeakReference<DebuggerProxyBase>(proxy);
}

public static void RegisterExitHandler(string id, Action<RunLoopExitState> handler)
{
if (s_exitHandlers.Any(t => t.id == id))
int intId = int.Parse(id);
if (s_exitHandlers.ContainsKey(intId))
throw new Exception($"Cannot register a duplicate exit handler for {id}");

s_exitHandlers.Add(new(id, handler));
s_exitHandlers[intId] = new WeakReference<Action<RunLoopExitState>>(handler);
}

public static void RegisterProxyExitState(string id, RunLoopExitState status)
{
Console.WriteLine ($"[{id}] RegisterProxyExitState: {status}");
s_statusTable.Add((id, status));
(string id, Action<RunLoopExitState> handler)[]? found = s_exitHandlers.Where(e => e.id == id).ToArray();
if (found.Length > 0)
found[0].handler.Invoke(status);
int intId = int.Parse(id);
s_statusTable[id] = status;
// we have the explicit state now, so we can drop the reference
// to the proxy
s_proxyTable.TryRemove(id, out WeakReference<DebuggerProxyBase> _);

if (s_exitHandlers.TryRemove(intId, out WeakReference<Action<RunLoopExitState>>? handlerRef)
&& handlerRef.TryGetTarget(out Action<RunLoopExitState>? handler))
{
handler(status);
}
}

// FIXME: remove
public static bool TryGetProxyExitState(string id, [NotNullWhen(true)] out RunLoopExitState? state)
{
state = new(RunLoopStopReason.Cancelled, null);
state = null;

if (!TryGetProxyById(id, out DebuggerProxyBase? proxy))
if (s_proxyTable.TryGetValue(id, out WeakReference<DebuggerProxyBase>? proxyRef) && proxyRef.TryGetTarget(out DebuggerProxyBase? proxy))
{
state = proxy.ExitState;
}
else if (!s_statusTable.TryGetValue(id, out state))
{
(string id, RunLoopExitState state)[]? found = s_statusTable.Where(t => t.id == id).ToArray();
if (found.Length == 0)
{
Console.WriteLine($"[{id}] Cannot find exit proxy for {id}");
return false;
}

state = found[0].state;
return true;
Console.WriteLine($"[{id}] Cannot find exit proxy for {id}");
state = null;
}

state = proxy.ExitState;
return state is not null;
}

public static DebuggerProxyBase? ShutdownProxy(string id)
{
if (!string.IsNullOrEmpty(id))
if (!string.IsNullOrEmpty(id)
&& s_proxyTable.TryGetValue(id, out WeakReference<DebuggerProxyBase>? proxyRef)
&& proxyRef.TryGetTarget(out DebuggerProxyBase? proxy))
{
(_, DebuggerProxyBase? proxy) = s_proxyTable.FirstOrDefault(t => t.id == id);
if (proxy is not null)
{
proxy.Shutdown();
return proxy;
}
proxy.Shutdown();
return proxy;
}
return null;
}
Expand Down

0 comments on commit f730657

Please sign in to comment.