diff --git a/CHANGELOG.txt b/CHANGELOG.txt index db8baaea..cd77c98a 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,10 @@ +v1.2.1 +- 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. - Remove site directory - Improve error messaging - Organize functions internally 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 diff --git a/src/choreographer/browser_async.py b/src/choreographer/browser_async.py index ed718f46..a0900a38 100644 --- a/src/choreographer/browser_async.py +++ b/src/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)) @@ -198,7 +204,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, ) @@ -250,6 +256,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() @@ -267,7 +275,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,