chore(roll): collect stale handles

microsoft · Oct 11, 2023 · eb4546f · eb4546f
1 parent 44a61ff
commit eb4546f
Show file tree

Hide file tree

Showing 31 changed files with 248 additions and 237 deletions.
diff --git a/src/Playwright.Tests/PageNetworkResponseTests.cs b/src/Playwright.Tests/PageNetworkResponseTests.cs
@@ -331,4 +331,19 @@ public async Task ShouldReportIfRequestWasFromServiceWorker()
             Assert.True(res.FromServiceWorker);
         }
     }
+
+    [PlaywrightTest("", "should collect stale handles")]
+    public async Task ShouldCollectStaleHandels()
+        {
+        Page.Request += (sender, request) => {};
+        var response = await Page.GotoAsync(Server.EmptyPage);
+        for (var i = 0; i < 1000; i++) {
+            await Page.EvaluateAsync(@"async () => {
+                const response = await fetch('/');
+                await response.text();
+            }");
+        }
+        var exception = await PlaywrightAssert.ThrowsAsync<PlaywrightException>(async () => await response.AllHeadersAsync());
+        StringAssert.Contains("The object has been collected to prevent unbounded heap growth.", exception.Message);
+    }
 }
diff --git a/src/Playwright/Core/Artifact.cs b/src/Playwright/Core/Artifact.cs
@@ -33,12 +33,10 @@ namespace Microsoft.Playwright.Core;
 
 internal class Artifact : ChannelOwnerBase, IChannelOwner<Artifact>
 {
-    private readonly Connection _connection;
     private readonly ArtifactChannel _channel;
 
     internal Artifact(IChannelOwner parent, string guid, ArtifactInitializer initializer) : base(parent, guid)
     {
-        _connection = parent.Connection;
         _channel = new(guid, parent.Connection, this);
         AbsolutePath = initializer.AbsolutePath;
     }

diff --git a/src/Playwright/Core/PlaywrightImpl.cs b/src/Playwright/Core/PlaywrightImpl.cs
@@ -36,15 +36,13 @@ internal class PlaywrightImpl : ChannelOwnerBase, IPlaywright, IChannelOwner<Pla
 {
     private readonly PlaywrightInitializer _initializer;
     internal readonly PlaywrightChannel _channel;
-    private readonly Connection _connection;
     internal SelectorsAPI _selectors;
 
     private readonly Dictionary<string, BrowserNewContextOptions> _devices = new(StringComparer.InvariantCultureIgnoreCase);
 
     internal PlaywrightImpl(IChannelOwner parent, string guid, PlaywrightInitializer initializer)
          : base(parent, guid)
     {
-        _connection = parent.Connection;
         _initializer = initializer;
         _channel = new(guid, parent.Connection, this);
 

diff --git a/src/Playwright/Core/Waiter.cs b/src/Playwright/Core/Waiter.cs
@@ -28,7 +28,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Playwright.Helpers;
-using Microsoft.Playwright.Transport.Channels;
+using Microsoft.Playwright.Transport;
 
 namespace Microsoft.Playwright.Core;
 
@@ -40,13 +40,13 @@ internal class Waiter : IDisposable
     private readonly CancellationTokenSource _onDisposeCts = new();
     private readonly CancellationTokenSource _manualCts = new();
     private readonly string _waitId = Guid.NewGuid().ToString();
-    private readonly IChannelOwner _channelOwner;
+    private readonly ChannelOwnerBase _channelOwner;
     private Exception _immediateError;
 
     private bool _disposed;
     private string _error;
 
-    internal Waiter(IChannelOwner channelOwner, string @event)
+    internal Waiter(ChannelOwnerBase channelOwner, string @event)
     {
         _channelOwner = channelOwner;
 
@@ -60,7 +60,7 @@ internal Waiter(IChannelOwner channelOwner, string @event)
             },
         };
         _failures.Add(Task.Delay(-1, _manualCts.Token));
-        _channelOwner.Connection.SendMessageToServerAsync(_channelOwner.Channel.Guid, "waitForEventInfo", beforeArgs).IgnoreException();
+        _channelOwner._connection.SendMessageToServerAsync(_channelOwner, "waitForEventInfo", beforeArgs).IgnoreException();
     }
 
     public void Dispose()
@@ -87,7 +87,7 @@ public void Dispose()
                 ["info"] = info,
             };
 
-            _channelOwner.WrapApiCallAsync(() => _channelOwner.Connection.SendMessageToServerAsync(_channelOwner.Channel.Guid, "waitForEventInfo", afterArgs), true).IgnoreException();
+            _channelOwner.WrapApiCallAsync(() => _channelOwner._connection.SendMessageToServerAsync(_channelOwner, "waitForEventInfo", afterArgs), true).IgnoreException();
 
             _onDisposeCts.Cancel();
             _onDisposeCts.Dispose();
@@ -109,7 +109,7 @@ internal void Log(string log)
                 ["message"] = log,
             },
         };
-        _channelOwner.WrapApiCallAsync(() => _channelOwner.Connection.SendMessageToServerAsync(_channelOwner.Channel.Guid, "waitForEventInfo", logArgs), true).IgnoreException();
+        _channelOwner.WrapApiCallAsync(() => _channelOwner._connection.SendMessageToServerAsync(_channelOwner, "waitForEventInfo", logArgs), true).IgnoreException();
     }
 
     internal void RejectImmediately(Exception exception)

diff --git a/src/Playwright/Transport/ChannelOwnerBase.cs b/src/Playwright/Transport/ChannelOwnerBase.cs
@@ -34,8 +34,9 @@ namespace Microsoft.Playwright.Transport;
 
 internal class ChannelOwnerBase : IChannelOwner
 {
-    private readonly Connection _connection;
+    internal readonly Connection _connection;
     private readonly ConcurrentDictionary<string, IChannelOwner> _objects = new();
+    internal bool _wasCollected;
 
     internal ChannelOwnerBase(IChannelOwner parent, string guid) : this(parent, null, guid)
     {
@@ -76,14 +77,15 @@ void IChannelOwner.Adopt(ChannelOwnerBase child)
     }
 
     /// <inheritdoc/>
-    void IChannelOwner.DisposeOwner()
+    void IChannelOwner.DisposeOwner(string reason)
     {
         Parent?.Objects?.TryRemove(Guid, out var _);
         _connection?.Objects.TryRemove(Guid, out var _);
+        _wasCollected = reason == "gc";
 
         foreach (var item in _objects.Values)
         {
-            item.DisposeOwner();
+            item.DisposeOwner(reason);
         }
         _objects.Clear();
     }
@@ -121,7 +123,7 @@ private void UpdateEventSubscription(string eventName, bool enabled)
     {
         WrapApiCallAsync(
             () => _connection.SendMessageToServerAsync(
-            Guid,
+            this,
             "updateSubscription",
             new Dictionary<string, object>
             {

diff --git a/src/Playwright/Transport/Channels/APIRequestContextChannel.cs b/src/Playwright/Transport/Channels/APIRequestContextChannel.cs
@@ -36,7 +36,7 @@ public APIRequestContextChannel(string guid, Connection connection, APIRequestCo
     {
     }
 
-    internal Task DisposeAsync() => Connection.SendMessageToServerAsync(Guid, "dispose");
+    internal Task DisposeAsync() => Connection.SendMessageToServerAsync(Object, "dispose");
 
     internal async Task<IAPIResponse> FetchAsync(
         string url,
@@ -68,16 +68,16 @@ internal async Task<IAPIResponse> FetchAsync(
             ["multipartData"] = multipartData?.ToProtocol(),
         };
 
-        var response = await Connection.SendMessageToServerAsync(Guid, "fetch", message, keepNulls: true).ConfigureAwait(false);
+        var response = await Connection.SendMessageToServerAsync(Object, "fetch", message, keepNulls: true).ConfigureAwait(false);
         return new Core.APIResponse(Object, response?.GetProperty("response").ToObject<Protocol.APIResponse>());
     }
 
     internal Task<StorageState> StorageStateAsync()
-        => Connection.SendMessageToServerAsync<StorageState>(Guid, "storageState", null);
+        => Connection.SendMessageToServerAsync<StorageState>(Object, "storageState", null);
 
     internal async Task<string> FetchResponseBodyAsync(string fetchUid)
     {
-        var response = await Connection.SendMessageToServerAsync(Guid, "fetchResponseBody", new Dictionary<string, object> { ["fetchUid"] = fetchUid }).ConfigureAwait(false);
+        var response = await Connection.SendMessageToServerAsync(Object, "fetchResponseBody", new Dictionary<string, object> { ["fetchUid"] = fetchUid }).ConfigureAwait(false);
         if (response?.TryGetProperty("binary", out var binary) == true)
         {
             return binary.ToString();
@@ -87,10 +87,10 @@ internal async Task<string> FetchResponseBodyAsync(string fetchUid)
 
     internal async Task<List<string>> FetchResponseLogAsync(string fetchUid)
     {
-        var response = await Connection.SendMessageToServerAsync(Guid, "fetchLog", new Dictionary<string, object> { ["fetchUid"] = fetchUid }).ConfigureAwait(false);
+        var response = await Connection.SendMessageToServerAsync(Object, "fetchLog", new Dictionary<string, object> { ["fetchUid"] = fetchUid }).ConfigureAwait(false);
         return response.Value.GetProperty("log").ToObject<List<string>>();
     }
 
     internal Task DisposeAPIResponseAsync(string fetchUid)
-        => Connection.SendMessageToServerAsync(Guid, "disposeAPIResponse", new Dictionary<string, object> { ["fetchUid"] = fetchUid });
+        => Connection.SendMessageToServerAsync(Object, "disposeAPIResponse", new Dictionary<string, object> { ["fetchUid"] = fetchUid });
 }
diff --git a/src/Playwright/Transport/Channels/ArtifactChannel.cs b/src/Playwright/Transport/Channels/ArtifactChannel.cs
@@ -38,14 +38,14 @@ public ArtifactChannel(string guid, Connection connection, Artifact owner) : bas
 
     internal async Task<string> PathAfterFinishedAsync()
         => (await Connection.SendMessageToServerAsync<JsonElement?>(
-            Guid,
+            Object,
             "pathAfterFinished",
             null)
             .ConfigureAwait(false)).GetString("value", true);
 
     internal Task SaveAsAsync(string path)
         => Connection.SendMessageToServerAsync<JsonElement>(
-            Guid,
+            Object,
             "saveAs",
             new Dictionary<string, object>
             {
@@ -54,32 +54,32 @@ internal Task SaveAsAsync(string path)
 
     internal async Task<Stream> SaveAsStreamAsync()
         => (await Connection.SendMessageToServerAsync<JsonElement>(
-            Guid,
+            Object,
             "saveAsStream",
             null)
             .ConfigureAwait(false)).GetObject<Stream>("stream", Connection);
 
     internal async Task<string> FailureAsync()
         => (await Connection.SendMessageToServerAsync<JsonElement?>(
-            Guid,
+            Object,
             "failure",
             null)
             .ConfigureAwait(false)).GetString("error", true);
 
     internal async Task<Stream> StreamAsync()
         => (await Connection.SendMessageToServerAsync<JsonElement?>(
-            Guid,
+            Object,
             "stream",
             null)
             .ConfigureAwait(false))?.GetObject<Stream>("stream", Connection);
 
     internal Task CancelAsync()
         => Connection.SendMessageToServerAsync<JsonElement>(
-            Guid,
+            Object,
             "cancel");
 
     internal Task DeleteAsync()
         => Connection.SendMessageToServerAsync<JsonElement>(
-            Guid,
+            Object,
             "delete");
 }
diff --git a/src/Playwright/Transport/Channels/BindingCallChannel.cs b/src/Playwright/Transport/Channels/BindingCallChannel.cs
@@ -39,7 +39,7 @@ public BindingCallChannel(string guid, Connection connection, BindingCall owner)
 
     internal Task RejectAsync(Exception error)
         => Connection.SendMessageToServerAsync<JsonElement>(
-            Guid,
+            Object,
             "reject",
             new Dictionary<string, object>
             {
@@ -48,7 +48,7 @@ internal Task RejectAsync(Exception error)
 
     internal Task ResolveAsync(object result)
         => Connection.SendMessageToServerAsync<JsonElement>(
-            Guid,
+            Object,
             "resolve",
             new Dictionary<string, object>
             {

diff --git a/src/Playwright/Transport/Channels/BrowserChannel.cs b/src/Playwright/Transport/Channels/BrowserChannel.cs
@@ -103,7 +103,7 @@ internal override void OnMessage(string method, JsonElement? serverParams)
 
     internal Task<CDPChannel> NewBrowserCDPSessionAsync()
     => Connection.SendMessageToServerAsync<CDPChannel>(
-        Guid,
+        Object,
         "newBrowserCDPSession");
 
     internal Task<BrowserContextChannel> NewContextAsync(
@@ -208,12 +208,12 @@ internal Task<BrowserContextChannel> NewContextAsync(
         }
 
         return Connection.SendMessageToServerAsync<BrowserContextChannel>(
-            Guid,
+            Object,
             "newContext",
             args);
     }
 
-    internal Task CloseAsync() => Connection.SendMessageToServerAsync<BrowserContextChannel>(Guid, "close", null);
+    internal Task CloseAsync() => Connection.SendMessageToServerAsync<BrowserContextChannel>(Object, "close", null);
 
     internal Task StartTracingAsync(IPage page, bool screenshots, string path, IEnumerable<string> categories)
     {
@@ -225,9 +225,9 @@ internal Task StartTracingAsync(IPage page, bool screenshots, string path, IEnum
             ["categories"] = categories,
         };
 
-        return Connection.SendMessageToServerAsync(Guid, "crStartTracing", args);
+        return Connection.SendMessageToServerAsync(Object, "crStartTracing", args);
     }
 
     internal async Task<string> StopTracingAsync()
-        => (await Connection.SendMessageToServerAsync(Guid, "crStopTracing", null).ConfigureAwait(false))?.GetProperty("binary").ToString();
+        => (await Connection.SendMessageToServerAsync(Object, "crStopTracing", null).ConfigureAwait(false))?.GetProperty("binary").ToString();
 }