Fix process leaks (#82)

Author: kblok

lib/PuppeteerSharp.Tests/Puppeteer/PuppeteerLaunchTests.cs

@@ -9,7 +9,7 @@
 namespace PuppeteerSharp.Tests.Puppeteer
 {
     [Collection("PuppeteerLoaderFixture collection")]
-    public class PuppeteerLaunchTests
+    public class PuppeteerLaunchTests : PuppeteerBaseTest
     {
         [Fact]
         public async Task ShouldSupportIgnoreHTTPSErrorsOption()
@@ -94,11 +94,12 @@ public async Task UserDataDirOption()
             var options = TestConstants.DefaultBrowserOptions();
             options.UserDataDir = userDataDir;
 
-            var browser = await PuppeteerSharp.Puppeteer.LaunchAsync(options, TestConstants.ChromiumRevision);
+            var launcher = new Launcher();
+            var browser = await launcher.LaunchAsync(options, TestConstants.ChromiumRevision);
             Assert.True(Directory.GetFiles(userDataDir).Length > 0);
             await browser.CloseAsync();
             Assert.True(Directory.GetFiles(userDataDir).Length > 0);
-            // Directory.Delete(userDataDir, true);
+            await launcher.TryDeleteUserDataDir();
         }
 
         [Fact]
@@ -108,11 +109,12 @@ public async Task UserDataDirArgument()
             var options = TestConstants.DefaultBrowserOptions();
             options.Args = options.Args.Concat(new[] { $"--user-data-dir={userDataDir}" }).ToArray();
 
-            var browser = await PuppeteerSharp.Puppeteer.LaunchAsync(options, TestConstants.ChromiumRevision);
+            var launcher = new Launcher();
+            var browser = await launcher.LaunchAsync(options, TestConstants.ChromiumRevision);
             Assert.True(Directory.GetFiles(userDataDir).Length > 0);
             await browser.CloseAsync();
             Assert.True(Directory.GetFiles(userDataDir).Length > 0);
-            // Directory.Delete(userDataDir, true);
+            await launcher.TryDeleteUserDataDir();
         }
 
         [Fact]
@@ -121,5 +123,21 @@ public void ShouldReturnTheDefaultChromeArguments()
             var args = PuppeteerSharp.Puppeteer.DefaultArgs;
             Assert.Contains("--no-first-run", args);
         }
+
+        [Fact]
+        public async Task ChromeShouldBeClosed()
+        {
+            var options = TestConstants.DefaultBrowserOptions();
+            var launcher = new Launcher();
+            var browser = await launcher.LaunchAsync(options, TestConstants.ChromiumRevision);
+            var page = await browser.NewPageAsync();
+
+            var response = await page.GoToAsync("https://www.google.com");
+            Assert.Equal(response.Status.ToString(), "OK");
+
+            await browser.CloseAsync();
+
+            Assert.True(launcher.IsChromeClosed);
+        }
     }
 }

lib/PuppeteerSharp.Tests/TestConstants.cs

@@ -16,7 +16,8 @@ public static class TestConstants
             Headless = Convert.ToBoolean(Environment.GetEnvironmentVariable("HEADLESS") ?? "true"),
             Args = new[] { "--no-sandbox" },
             Timeout = 0,
-            KeepAliveInterval = 120
+            KeepAliveInterval = 120,
+            LogProcess = true
         };
     }
 }

lib/PuppeteerSharp/Connection.cs

@@ -34,6 +34,9 @@ public Connection(string url, int delay, ClientWebSocket ws)
         private Dictionary<string, Session> _sessions;
         private bool _closed = false;
         private TaskCompletionSource<bool> _connectionCloseTask;
+
+        private bool _closeMessageSent;
+        private const string CloseMessage = "Browser.close";
         #endregion
 
         #region Properties
@@ -59,6 +62,14 @@ public async Task<dynamic> SendAsync(string method, dynamic args = null)
             var buffer = new ArraySegment<Byte>(encoded, 0, encoded.Length);
             QueueId(id);
             await WebSocket.SendAsync(buffer, WebSocketMessageType.Text, true, default(CancellationToken));
+
+            //I don't know if this will be the final solution.
+            //For now this will prevent the WebSocket from failing after the process is killed by the close method.
+            if (method == CloseMessage)
+            {
+                _closeMessageSent = true;
+            }
+
             return await _responses[id].Task;
         }
 
@@ -80,13 +91,16 @@ public async Task<Session> CreateSession(string targetId)
 
         private void OnClose()
         {
-            _closed = true;
-            _connectionCloseTask.SetResult(true);
+            if (!_closed)
+            {
+                _closed = true;
+                _connectionCloseTask.SetResult(true);
 
-            Closed?.Invoke(this, new EventArgs());
+                Closed?.Invoke(this, new EventArgs());
 
-            _responses.Clear();
-            _sessions.Clear();
+                _responses.Clear();
+                _sessions.Clear();
+            }
         }
 
         /// <summary>
@@ -102,6 +116,7 @@ private async Task<object> GetResponseAsync()
             {
                 if (_closed)
                 {
+                    OnClose();
                     return null;
                 }
 
@@ -119,10 +134,23 @@ await Task.WhenAny(
 
                     if (_closed)
                     {
+                        OnClose();
                         return null;
                     }
 
-                    var result = socketTask.Result;
+                    WebSocketReceiveResult result = null;
+                    try
+                    {
+                        result = socketTask.Result;
+                    }
+                    catch (AggregateException) when (_closeMessageSent)
+                    {
+                        if (!_closed)
+                        {
+                            OnClose();
+                            return null;
+                        }
+                    }
 
                     endOfMessage = result.EndOfMessage;
 

lib/PuppeteerSharp/LaunchOptions.cs

@@ -28,5 +28,7 @@ public class LaunchOptions
         public bool Devtools { get; set; }
 
         public int KeepAliveInterval { get; set; } = 30;
+
+        public bool LogProcess { get; set; }
     }
 }