From 3838bff397f3b8a028bfa6e651d2e1a2bf8a66db Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Thu, 1 Dec 2022 23:08:28 +0100 Subject: [PATCH] BrowserSteps - More work on cleaner shutdowns of browser session --- .../blueprint/browser_steps/__init__.py | 59 ++++++++----------- .../blueprint/browser_steps/nonContext.py | 18 ++++++ requirements.txt | 3 - 3 files changed, 41 insertions(+), 39 deletions(-) create mode 100644 changedetectionio/blueprint/browser_steps/nonContext.py diff --git a/changedetectionio/blueprint/browser_steps/__init__.py b/changedetectionio/blueprint/browser_steps/__init__.py index e4112521..9cd17aae 100644 --- a/changedetectionio/blueprint/browser_steps/__init__.py +++ b/changedetectionio/blueprint/browser_steps/__init__.py @@ -21,9 +21,6 @@ # OR # - use multiprocessing to bump this over to its own process and add some transport layer (queue/pipes) - - - from distutils.util import strtobool from flask import Blueprint, request, make_response from flask_login import login_required @@ -33,42 +30,35 @@ from changedetectionio.store import ChangeDetectionStore browsersteps_live_ui_o = {} browsersteps_playwright_browser_interface = None -browsersteps_playwright_browser_interface_start_time = None browsersteps_playwright_browser_interface_browser = None +browsersteps_playwright_browser_interface_context = None browsersteps_playwright_browser_interface_end_time = None - +browsersteps_playwright_browser_interface_start_time = None def cleanup_playwright_session(): - print("Cleaning up old playwright session because time was up") - global browsersteps_playwright_browser_interface + global browsersteps_live_ui_o - global browsersteps_playwright_browser_interface_browser global browsersteps_playwright_browser_interface - global browsersteps_playwright_browser_interface_start_time + global browsersteps_playwright_browser_interface_browser + global browsersteps_playwright_browser_interface_context global browsersteps_playwright_browser_interface_end_time - - import psutil - - # playwright's .stop() seems to hang, so lets kill the processes before calling .stop - # a bit hard, but more reliable :/ - current_process = psutil.Process() - children = current_process.children(recursive=True) - for child in children: - # @todo a bug here is that we could be accidently shutting down the playwright from the content checker - if "playwright" in child.name() or child.name() == "node": - print(child) - print('Child pid is {}, killing'.format(child.pid)) - child.terminate() - - print ("Shutting down playwright interface .stop()") - browsersteps_playwright_browser_interface.stop() - print ("Shutdown of playwright complete") + global browsersteps_playwright_browser_interface_start_time browsersteps_live_ui_o = {} - browsersteps_playwright_browser_interface = None - browsersteps_playwright_browser_interface_start_time = None browsersteps_playwright_browser_interface_browser = None browsersteps_playwright_browser_interface_end_time = None + browsersteps_playwright_browser_interface_start_time = None + print("Cleaning up old playwright session because time was up, calling .goodbye()") + + try: + browsersteps_playwright_browser_interface_context.goodbye() + except Exception as e: + print ("Got exception in shutdown, probably OK") + print (str(e)) + + browsersteps_playwright_browser_interface = None + browsersteps_playwright_browser_interface_context = None + print ("Cleaning up old playwright session because time was up - done") def construct_blueprint(datastore: ChangeDetectionStore): @@ -103,12 +93,8 @@ def construct_blueprint(datastore: ChangeDetectionStore): if browsersteps_playwright_browser_interface_end_time: remaining = browsersteps_playwright_browser_interface_end_time-time.time() if browsersteps_playwright_browser_interface_end_time and remaining <= 0: - - cleanup_playwright_session() - - return make_response('Browser session expired, please reload the Browser Steps interface', 500) - + return make_response('Browser session expired, please reload the Browser Steps interface', 401) # Actions - step/apply/etc, do the thing and return state if request.method == 'POST': @@ -156,10 +142,11 @@ def construct_blueprint(datastore: ChangeDetectionStore): if not browsersteps_playwright_browser_interface: print("Starting connection with playwright") logging.debug("browser_steps.py connecting") - from playwright.sync_api import sync_playwright - - browsersteps_playwright_browser_interface = sync_playwright().start() + global browsersteps_playwright_browser_interface_context + from . import nonContext + browsersteps_playwright_browser_interface_context = nonContext.c_sync_playwright() + browsersteps_playwright_browser_interface = browsersteps_playwright_browser_interface_context.start() time.sleep(1) # At 20 minutes, some other variable is closing it diff --git a/changedetectionio/blueprint/browser_steps/nonContext.py b/changedetectionio/blueprint/browser_steps/nonContext.py new file mode 100644 index 00000000..5345f306 --- /dev/null +++ b/changedetectionio/blueprint/browser_steps/nonContext.py @@ -0,0 +1,18 @@ +from playwright.sync_api import PlaywrightContextManager +import asyncio + +# So playwright wants to run as a context manager, but we do something horrible and hacky +# we are holding the session open for as long as possible, then shutting it down, and opening a new one +# So it means we don't get to use PlaywrightContextManager' __enter__ __exit__ +# To work around this, make goodbye() act the same as the __exit__() +# +# But actually I think this is because the context is opened correctly with __enter__() but we timeout the connection +# then theres some lock condition where we cant destroy it without it hanging + +class c_PlaywrightContextManager(PlaywrightContextManager): + + def goodbye(self) -> None: + self.__exit__() + +def c_sync_playwright() -> PlaywrightContextManager: + return c_PlaywrightContextManager() diff --git a/requirements.txt b/requirements.txt index c1534fd4..70eb4208 100644 --- a/requirements.txt +++ b/requirements.txt @@ -58,6 +58,3 @@ jq~=1.3 ;python_version >= "3.8" and sys_platform == "linux" # Any current modern version, required so far for screenshot PNG->JPEG conversion but will be used more in the future pillow # playwright is installed at Dockerfile build time because it's not available on all platforms - -# For shutting down playwright BrowserSteps nicely -psutil \ No newline at end of file