From 2df2769409efb56570aa1b14350949f92a769c90 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 31 Oct 2025 14:54:06 -0500 Subject: [PATCH 1/4] Move all process functions to new threadpool executor. --- choreographer/browser_async.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/choreographer/browser_async.py b/choreographer/browser_async.py index d124221d..80913e5d 100644 --- a/choreographer/browser_async.py +++ b/choreographer/browser_async.py @@ -46,6 +46,9 @@ async def close(self) -> None: class Browser(Target): """`Browser` is the async implementation of `Browser`.""" + subprocess: subprocess.Popen[bytes] | subprocess.Popen[str] + """A reference to the `Popen` object.""" + tabs: MutableMapping[str, Tab] """A mapping by target_id of all the targets which are open tabs.""" targets: MutableMapping[str, Target] @@ -96,7 +99,7 @@ def __init__( """ _logger.debug("Attempting to open new browser.") - self._check_closed_executor = _manual_thread_pool.ManualThreadExecutor( + self._process_executor = _manual_thread_pool.ManualThreadExecutor( max_workers=3, name="checking_close", ) @@ -144,7 +147,10 @@ def run() -> subprocess.Popen[bytes] | subprocess.Popen[str]: # depends on args _logger.debug("Trying to open browser.") loop = asyncio.get_running_loop() - self.subprocess = await loop.run_in_executor(None, run) + self.subprocess = await loop.run_in_executor( + self._process_executor, + run, + ) super().__init__("0", self._broker) self._add_session(Session("", self._broker)) @@ -194,7 +200,7 @@ async def _is_closed(self, wait: int | None = 0) -> bool: loop = asyncio.get_running_loop() await loop.run_in_executor( - self._check_closed_executor, + self._process_executor, self.subprocess.wait, wait, ) @@ -246,6 +252,8 @@ async def close(self) -> None: self._watch_dog_task.cancel() if not self._release_lock(): return + # it can never be mid open here, because all of these must + # run on the same thread. Do not push open or close to threads. try: _logger.debug("Starting browser close methods.") await self._close() @@ -263,7 +271,7 @@ async def close(self) -> None: _logger.debug("Browser channel closed.") self._browser_impl.clean() # os blocky/hangy across networks _logger.debug("Browser implementation cleaned up.") - self._check_closed_executor.shutdown(wait=False, cancel_futures=True) + self._process_executor.shutdown(wait=False, cancel_futures=True) async def __aexit__( self, From f7d2227f9d89cae85cd41944ac1365374803f50d Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 31 Oct 2025 15:05:46 -0500 Subject: [PATCH 2/4] Push changelog. --- CHANGELOG.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 95b039a3..6b5dc485 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,5 @@ +v1.2.1 +- Use custom ThreadpoolExecutor for all browser-related actions v1.2.0 - Delete zipfile after downloading - Upgrade logistro to reduce sideeffects From 87f10589fb2c0e47b5b3a03dd4393f27ff4a9042 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Sun, 9 Nov 2025 12:47:53 -0500 Subject: [PATCH 3/4] Further explain changelog. --- CHANGELOG.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 6b5dc485..ac703dc3 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,5 +1,10 @@ v1.2.1 -- Use custom ThreadpoolExecutor for all browser-related actions +- Use custom threadpool for functions that could be running during shutdown: + Python's stdlib threadpool isn't available during interpreter shutdown, nor + `atexit`- so they cannot be started or shutdown during `atexit`, or relied + upon at all. We use a custom threadpool during `Browser.close()` so that it + can be leveraged during atexit, and now we use it during `Browser.open()` + since certain use patterns have that function running during shutdown. v1.2.0 - Delete zipfile after downloading - Upgrade logistro to reduce sideeffects From d436b7dc12e9a2452d1de2653f5269fe97639f49 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Sun, 9 Nov 2025 13:55:35 -0500 Subject: [PATCH 4/4] Update ROADMAP.md --- ROADMAP.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 5b6346b2..6ddeb626 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -3,11 +3,10 @@ - [ ] Working on better diagnostic information - [ ] Explain to user when their system has security restrictions - [ ] Eliminate synchronous API: it's unused, hard to maintain, and nearly -worthless + worthless - [ ] Diagnose function should collect a JSON and then print that - [ ] Allow user to build and send their own JSONS - [ ] Get serialization out of the lock -- [ ] - [ ] Support Firefox - [ ] Support LadyBird (!) - [ ] Test against multiple docker containers