From f66337fa6abbd165a9445cf7423017b491889248 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:08:04 -0500 Subject: [PATCH 1/8] Add single-process to flags --- choreographer/browsers/chromium.py | 1 + 1 file changed, 1 insertion(+) diff --git a/choreographer/browsers/chromium.py b/choreographer/browsers/chromium.py index f297d7fe..c0706b7e 100644 --- a/choreographer/browsers/chromium.py +++ b/choreographer/browsers/chromium.py @@ -212,6 +212,7 @@ def get_cli(self) -> Sequence[str]: "--disable-print-preview", "--disable-speech-api", "--mute-audio", + "--single-process", "--no-default-browser-check", "--no-pings", "--disable-features=Translate,BackForwardCache,AcceptCHFrame,MediaRouter,OptimizationHints,AudioServiceOutOfProcess,IsolateOrigins,site-per-process", From 816fb90626a8fe83f58f99d45b583590d24f93d3 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:16:50 -0500 Subject: [PATCH 2/8] Remove cli flag --- choreographer/browsers/chromium.py | 1 - 1 file changed, 1 deletion(-) diff --git a/choreographer/browsers/chromium.py b/choreographer/browsers/chromium.py index c0706b7e..28236a43 100644 --- a/choreographer/browsers/chromium.py +++ b/choreographer/browsers/chromium.py @@ -195,7 +195,6 @@ def get_cli(self) -> Sequence[str]: "--disable-dev-shm-usage", "--disable-background-networking", "--disable-background-timer-throttling", - "--disable-backgrounding-occluded-windows", "--disable-component-update", "--disable-default-apps", "--disable-extensions", From f695dca4d0db53530ccdf91f29c27fac4d0ac897 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:18:55 -0500 Subject: [PATCH 3/8] Further extend timeout --- choreographer/browsers/chromium.py | 2 +- choreographer/utils/_tmpfile.py | 2 +- tests/conftest.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/choreographer/browsers/chromium.py b/choreographer/browsers/chromium.py index 28236a43..f297d7fe 100644 --- a/choreographer/browsers/chromium.py +++ b/choreographer/browsers/chromium.py @@ -195,6 +195,7 @@ def get_cli(self) -> Sequence[str]: "--disable-dev-shm-usage", "--disable-background-networking", "--disable-background-timer-throttling", + "--disable-backgrounding-occluded-windows", "--disable-component-update", "--disable-default-apps", "--disable-extensions", @@ -211,7 +212,6 @@ def get_cli(self) -> Sequence[str]: "--disable-print-preview", "--disable-speech-api", "--mute-audio", - "--single-process", "--no-default-browser-check", "--no-pings", "--disable-features=Translate,BackForwardCache,AcceptCHFrame,MediaRouter,OptimizationHints,AudioServiceOutOfProcess,IsolateOrigins,site-per-process", diff --git a/choreographer/utils/_tmpfile.py b/choreographer/utils/_tmpfile.py index bb9a32f5..23a46f11 100644 --- a/choreographer/utils/_tmpfile.py +++ b/choreographer/utils/_tmpfile.py @@ -193,7 +193,7 @@ def extra_clean() -> None: _logger.info(f"Extra manual clean executing {i}.") self._delete_manually(quiet=True) i += 1 - time.sleep(2) + time.sleep(10) if self.path.exists(): self._delete_manually(quiet=False) diff --git a/tests/conftest.py b/tests/conftest.py index ee8388e3..d57b34fc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -89,7 +89,7 @@ async def wrapped_test_fn(*args, **kwargs): def pytest_configure(): # change this by command line TODO - pytest.default_timeout = 20 + pytest.default_timeout = 60 # pytest shuts down its capture before logging/threads finish From 65a96806b070b0d5c243b625a16de9ae1a7ee4a0 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:20:43 -0500 Subject: [PATCH 4/8] Modify windows kill to go after tree --- choreographer/utils/_kill.py | 2 +- tests/test_process.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/choreographer/utils/_kill.py b/choreographer/utils/_kill.py index d7e43442..4c29b1f5 100644 --- a/choreographer/utils/_kill.py +++ b/choreographer/utils/_kill.py @@ -7,7 +7,7 @@ def kill(process: subprocess.Popen[bytes]) -> None: if platform.system() == "Windows": subprocess.call( # noqa: S603, false positive, input fine - ["taskkill", "/F", "/T", "/PID", str(process.pid)], # noqa: S607 windows full path... + ["taskkill", "/IM", "/F", "/T", "/PID", str(process.pid)], # noqa: S607 windows full path... stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, ) diff --git a/tests/test_process.py b/tests/test_process.py index b32733c0..bc2b65dd 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -77,7 +77,7 @@ async def test_watchdog(headless): if platform.system() == "Windows": # Blocking process here because it ensures the kill will occur rn subprocess.call( # noqa: S603, ASYNC221 sanitize input, blocking process - ["taskkill", "/F", "/T", "/PID", str(browser.subprocess.pid)], # noqa: S607 + ["taskkill", "/IM", "/F", "/T", "/PID", str(browser.subprocess.pid)], # noqa: S607 stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, ) From 4313b909ae8879e1ded751f5d2f13701ea1933a9 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:27:28 -0500 Subject: [PATCH 5/8] Add logging to kill --- choreographer/utils/_kill.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/choreographer/utils/_kill.py b/choreographer/utils/_kill.py index 4c29b1f5..ed32a821 100644 --- a/choreographer/utils/_kill.py +++ b/choreographer/utils/_kill.py @@ -3,6 +3,10 @@ import platform import subprocess +import logistro + +_logger = logistro.getLogger(__name__) + def kill(process: subprocess.Popen[bytes]) -> None: if platform.system() == "Windows": @@ -13,5 +17,7 @@ def kill(process: subprocess.Popen[bytes]) -> None: ) else: process.terminate() + _logger.info("Called terminate") if process.poll() is None: + _logger.info("Calling kill") process.kill() From 5e87b7a8255a23ca366029c2bcb76755af929395 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:31:16 -0500 Subject: [PATCH 6/8] Use old taskkill flags --- choreographer/utils/_kill.py | 2 +- tests/test_process.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/choreographer/utils/_kill.py b/choreographer/utils/_kill.py index ed32a821..b2762c76 100644 --- a/choreographer/utils/_kill.py +++ b/choreographer/utils/_kill.py @@ -11,7 +11,7 @@ def kill(process: subprocess.Popen[bytes]) -> None: if platform.system() == "Windows": subprocess.call( # noqa: S603, false positive, input fine - ["taskkill", "/IM", "/F", "/T", "/PID", str(process.pid)], # noqa: S607 windows full path... + ["taskkill", "/F", "/T", "/PID", str(process.pid)], # noqa: S607 windows full path... stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, ) diff --git a/tests/test_process.py b/tests/test_process.py index bc2b65dd..b32733c0 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -77,7 +77,7 @@ async def test_watchdog(headless): if platform.system() == "Windows": # Blocking process here because it ensures the kill will occur rn subprocess.call( # noqa: S603, ASYNC221 sanitize input, blocking process - ["taskkill", "/IM", "/F", "/T", "/PID", str(browser.subprocess.pid)], # noqa: S607 + ["taskkill", "/F", "/T", "/PID", str(browser.subprocess.pid)], # noqa: S607 stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, ) From 18b35c9e5ac373da4e76794c84df7748836ebd6b Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:32:59 -0500 Subject: [PATCH 7/8] Give browser time to close --- choreographer/browser_async.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/choreographer/browser_async.py b/choreographer/browser_async.py index 29a36214..4c86068a 100644 --- a/choreographer/browser_async.py +++ b/choreographer/browser_async.py @@ -179,7 +179,7 @@ async def _close(self) -> None: return except ChannelClosedError: _logger.debug("Can send browser.close on close channel") - + await asyncio.sleep(0.3) await asyncio.to_thread(self._channel.close) if await self._is_closed(): From dd5ab73e73f682e064aa393907cc390c065287b4 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Wed, 29 Jan 2025 00:36:14 -0500 Subject: [PATCH 8/8] Add wait to after sneding browser.close() --- choreographer/browser_async.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/choreographer/browser_async.py b/choreographer/browser_async.py index 4c86068a..b6ea5ab6 100644 --- a/choreographer/browser_async.py +++ b/choreographer/browser_async.py @@ -179,8 +179,9 @@ async def _close(self) -> None: return except ChannelClosedError: _logger.debug("Can send browser.close on close channel") - await asyncio.sleep(0.3) await asyncio.to_thread(self._channel.close) + if await self._is_closed(wait=2): + return if await self._is_closed(): _logger.debug("Browser is closed after closing channel")