Merge remote-tracking branch 'origin/main' into close-inactive-contexts

Author: elacuesta

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.0.36
+current_version = 0.0.37
 commit = True
 tag = True
 

.github/workflows/checks.yml

@@ -5,33 +5,29 @@ jobs:
   checks:
     if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository
     runs-on: ubuntu-latest
+    timeout-minutes: 5
     strategy:
       fail-fast: false
       matrix:
         include:
-        - python-version: 3.8
-          env:
+        - env:
             TOXENV: bandit
-        - python-version: 3.8
-          env:
+        - env:
             TOXENV: black
-        - python-version: 3.8
-          env:
+        - env:
             TOXENV: flake8
-        - python-version: 3.8
-          env:
+        - env:
             TOXENV: typing
-        - python-version: 3.8
-          env:
+        - env:
             TOXENV: pylint
 
     steps:
     - uses: actions/checkout@v4
 
-    - name: Set up Python ${{ matrix.python-version }}
+    - name: Set up Python
       uses: actions/setup-python@v5
       with:
-        python-version: ${{ matrix.python-version }}
+        python-version: 3.11
 
     - name: Run check
       env: ${{ matrix.env }}

.github/workflows/publish.yml

@@ -6,14 +6,15 @@ on:
 jobs:
   publish:
     runs-on: ubuntu-latest
+    timeout-minutes: 5
 
     steps:
     - uses: actions/checkout@v4
 
-    - name: Set up Python 3
+    - name: Set up Python
       uses: actions/setup-python@v5
       with:
-        python-version: 3
+        python-version: 3.11
 
     - name: Publish to PyPI
       run: |

.github/workflows/tests.yml

@@ -5,6 +5,7 @@ jobs:
   tests:
     if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository
     runs-on: ${{ matrix.os }}
+    timeout-minutes: 20
     strategy:
       fail-fast: false
       matrix:

README.md

@@ -1014,7 +1014,12 @@ asyncio.run(main())
 
 ### Software versions
 
-Be sure to include which versions of Scrapy and scrapy-playwright you are using:
+Be sure to include which versions of Scrapy, Playwright and scrapy-playwright you are using:
+
+```
+$ playwright --version
+Version 1.44.0
+```
 
 ```
 $ python -c "import scrapy_playwright; print(scrapy_playwright.__version__)"
@@ -1067,6 +1072,12 @@ class ExampleSpider(scrapy.Spider):
         )
 ```
 
+#### Minimal code
+Please make the effort to reduce the code to the minimum that still displays the issue.
+It is very rare that a complete project (including middlewares, pipelines, item processing, etc)
+is really needed to reproduce an issue. Reports that do not show an actual debugging attempt
+will not be considered.
+
 ### Logs and stats
 
 Logs for spider jobs displaying the issue in detail are extremely useful

docs/changelog.md

@@ -1,5 +1,10 @@
 # scrapy-playwright changelog
 
+### [v0.0.37](https://github.com/scrapy-plugins/scrapy-playwright/releases/tag/v0.0.37) (2024-07-03)
+
+* Improve Windows concurrency (#286)
+
+
 ### [v0.0.36](https://github.com/scrapy-plugins/scrapy-playwright/releases/tag/v0.0.36) (2024-06-24)
 
 * Windows support (#276)

scrapy_playwright/__init__.py

@@ -1 +1 @@
-__version__ = "0.0.36"
+__version__ = "0.0.37"

scrapy_playwright/_utils.py

@@ -1,5 +1,4 @@
 import asyncio
-import concurrent
 import logging
 import platform
 import threading
@@ -106,34 +105,66 @@ async def _get_header_value(
 
 if platform.system() == "Windows":
 
-    class _WindowsAdapter:
-        """Utility class to redirect coroutines to an asyncio event loop running
-        in a different thread. This allows to use a ProactorEventLoop, which is
-        supported by Playwright on Windows.
+    class _ThreadedLoopAdapter:
+        """Utility class to start an asyncio event loop in a new thread and redirect coroutines.
+        This allows to run Playwright in a different loop than the Scrapy crawler, allowing to
+        use ProactorEventLoop which is supported by Playwright on Windows.
         """
 
-        loop = None
-        thread = None
+        _loop: asyncio.AbstractEventLoop
+        _thread: threading.Thread
+        _coro_queue: asyncio.Queue = asyncio.Queue()
+        _stop_event: asyncio.Event = asyncio.Event()
 
         @classmethod
-        def get_event_loop(cls) -> asyncio.AbstractEventLoop:
-            if cls.thread is None:
-                if cls.loop is None:
-                    policy = asyncio.WindowsProactorEventLoopPolicy()  # type: ignore
-                    cls.loop = policy.new_event_loop()
-                    asyncio.set_event_loop(cls.loop)
-                if not cls.loop.is_running():
-                    cls.thread = threading.Thread(target=cls.loop.run_forever, daemon=True)
-                    cls.thread.start()
-                    logger.info("Started loop on separate thread: %s", cls.loop)
-            return cls.loop
+        async def _handle_coro(cls, coro, future) -> None:
+            try:
+                future.set_result(await coro)
+            except Exception as exc:
+                future.set_exception(exc)
 
         @classmethod
-        async def get_result(cls, coro) -> concurrent.futures.Future:
-            return asyncio.run_coroutine_threadsafe(coro=coro, loop=cls.get_event_loop()).result()
+        async def _process_queue(cls) -> None:
+            while not cls._stop_event.is_set():
+                coro, future = await cls._coro_queue.get()
+                asyncio.create_task(cls._handle_coro(coro, future))
+                cls._coro_queue.task_done()
 
-    def _deferred_from_coro(coro) -> Deferred:
-        return scrapy.utils.defer.deferred_from_coro(_WindowsAdapter.get_result(coro))
+        @classmethod
+        def _deferred_from_coro(cls, coro) -> Deferred:
+            future: asyncio.Future = asyncio.Future()
+            asyncio.run_coroutine_threadsafe(cls._coro_queue.put((coro, future)), cls._loop)
+            return scrapy.utils.defer.deferred_from_coro(future)
+
+        @classmethod
+        def start(cls) -> None:
+            policy = asyncio.WindowsProactorEventLoopPolicy()  # type: ignore[attr-defined]
+            cls._loop = policy.new_event_loop()
+            asyncio.set_event_loop(cls._loop)
+
+            cls._thread = threading.Thread(target=cls._loop.run_forever, daemon=True)
+            cls._thread.start()
+            logger.info("Started loop on separate thread: %s", cls._loop)
+
+            asyncio.run_coroutine_threadsafe(cls._process_queue(), cls._loop)
+
+        @classmethod
+        def stop(cls) -> None:
+            cls._stop_event.set()
+            asyncio.run_coroutine_threadsafe(cls._coro_queue.join(), cls._loop)
+            cls._loop.call_soon_threadsafe(cls._loop.stop)
+            cls._thread.join()
 
+    _deferred_from_coro = _ThreadedLoopAdapter._deferred_from_coro
 else:
+
+    class _ThreadedLoopAdapter:  # type: ignore[no-redef]
+        @classmethod
+        def start(cls) -> None:
+            pass
+
+        @classmethod
+        def stop(cls) -> None:
+            pass
+
     _deferred_from_coro = scrapy.utils.defer.deferred_from_coro

scrapy_playwright/handler.py

@@ -33,6 +33,7 @@
 from scrapy_playwright.headers import use_scrapy_headers
 from scrapy_playwright.page import PageMethod
 from scrapy_playwright._utils import (
+    _ThreadedLoopAdapter,
     _deferred_from_coro,
     _encode_body,
     _get_float_setting,
@@ -109,6 +110,7 @@ class ScrapyPlaywrightDownloadHandler(HTTPDownloadHandler):
 
     def __init__(self, crawler: Crawler) -> None:
         super().__init__(settings=crawler.settings, crawler=crawler)
+        _ThreadedLoopAdapter.start()
         if platform.system() != "Windows":
             verify_installed_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor")
         crawler.signals.connect(self._engine_started, signals.engine_started)
@@ -324,6 +326,7 @@ def close(self) -> Deferred:
         logger.info("Closing download handler")
         yield super().close()
         yield _deferred_from_coro(self._close())
+        _ThreadedLoopAdapter.stop()
 
     async def _close(self) -> None:
         logger.info("Closing %i contexts", len(self.context_wrappers))

tests/__init__.py

@@ -1,3 +1,4 @@
+import asyncio
 import inspect
 import logging
 import platform
@@ -13,17 +14,19 @@
 
 
 if platform.system() == "Windows":
-    from scrapy_playwright._utils import _WindowsAdapter
+    from scrapy_playwright._utils import _ThreadedLoopAdapter
 
     def allow_windows(test_method):
-        """Wrap tests with the _WindowsAdapter class on Windows."""
+        """Wrap tests with the _ThreadedLoopAdapter class on Windows."""
         if not inspect.iscoroutinefunction(test_method):
             raise RuntimeError(f"{test_method} must be an async def method")
 
         @wraps(test_method)
         async def wrapped(self, *args, **kwargs):
-            logger.debug("Calling _WindowsAdapter.get_result for %r", self)
-            await _WindowsAdapter.get_result(test_method(self, *args, **kwargs))
+            _ThreadedLoopAdapter.start()
+            coro = test_method(self, *args, **kwargs)
+            asyncio.run_coroutine_threadsafe(coro=coro, loop=_ThreadedLoopAdapter._loop).result()
+            _ThreadedLoopAdapter.stop()
 
         return wrapped