Merge 2924449fa1 into d8ba1571e7

docs/topics/asyncio.rst

@@ -221,7 +221,8 @@ for its differences and limitations compared to
 
 Additionally, :class:`~scrapy.crawler.AsyncCrawlerProcess` will install a
 :term:`meta path finder` that prevents :mod:`twisted.internet.reactor` from
-being imported.
+being imported. It will be uninstalled when :meth:`AsyncCrawlerProcess.start()
+<scrapy.crawler.AsyncCrawlerProcess.start>` returns.
 
 .. _asyncio-without-reactor-migrate:
 

scrapy/crawler.py

@@ -39,7 +39,11 @@ from scrapy.utils.reactor import (
     verify_installed_asyncio_event_loop,
     verify_installed_reactor,
 )
-from scrapy.utils.reactorless import install_reactor_import_hook
+from scrapy.utils.reactorless import (
+    ReactorImportHook,
+    install_reactor_import_hook,
+    uninstall_reactor_import_hook,
+)
 
 if TYPE_CHECKING:
     from collections.abc import Awaitable, Generator, Iterable
@@ -837,6 +841,7 @@ class AsyncCrawlerProcess(CrawlerProcessBase, AsyncCrawlerRunner):
         super().__init__(settings, install_root_handler)
         logger.debug("Using AsyncCrawlerProcess")
         self._reactorless_loop: asyncio.AbstractEventLoop | None = None
+        self._reactor_import_hook: ReactorImportHook | None = None
         # We want the asyncio event loop to be installed early, so that it's
         # always the correct one. And as we do that, we can also install the
         # reactor here.
@@ -849,7 +854,7 @@ class AsyncCrawlerProcess(CrawlerProcessBase, AsyncCrawlerRunner):
                     "TWISTED_REACTOR_ENABLED is False but a Twisted reactor is installed."
                 )
             self._reactorless_loop = set_asyncio_event_loop(loop_path)
-            install_reactor_import_hook()
+            self._reactor_import_hook = install_reactor_import_hook()
         elif is_reactor_installed():
             # The user could install a reactor before this class is instantiated.
             # We need to make sure the reactor is the correct one and the loop
@@ -983,6 +988,9 @@ class AsyncCrawlerProcess(CrawlerProcessBase, AsyncCrawlerRunner):
             asyncio.set_event_loop(None)
             loop.close()
             self._reactorless_loop = None
+            if self._reactor_import_hook:
+                uninstall_reactor_import_hook(self._reactor_import_hook)
+                self._reactor_import_hook = None
 
     @staticmethod
     def _cancel_all_tasks(loop: asyncio.AbstractEventLoop) -> None:

scrapy/utils/reactorless.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import contextlib
 import sys
 from importlib.abc import MetaPathFinder
 from typing import TYPE_CHECKING
@@ -47,7 +48,19 @@ class ReactorImportHook(MetaPathFinder):
         return None
 
 
-def install_reactor_import_hook() -> None:
-    """Prevent importing :mod:`twisted.internet.reactor`."""
+def install_reactor_import_hook() -> ReactorImportHook:
+    """Prevent importing :mod:`twisted.internet.reactor`.
 
-    sys.meta_path.insert(0, ReactorImportHook())
+    The hook is returned and can later be uninstalled with
+    :func:`uninstall_reactor_import_hook()`.
+    """
+
+    hook = ReactorImportHook()
+    sys.meta_path.insert(0, hook)
+    return hook
+
+
+def uninstall_reactor_import_hook(hook: ReactorImportHook) -> None:
+    """Uninstall the hook installed with :func:`install_reactor_import_hook()`."""
+    with contextlib.suppress(ValueError):
+        sys.meta_path.remove(hook)

tests/AsyncCrawlerProcess/reactorless_import_hook_multiple.py

@@ -0,0 +1,29 @@
+import logging
+import sys
+
+import scrapy
+from scrapy.crawler import AsyncCrawlerProcess
+from scrapy.utils.reactorless import ReactorImportHook
+
+logger = logging.getLogger(__name__)
+
+
+def hook_count() -> int:
+    return sum(1 for finder in sys.meta_path if isinstance(finder, ReactorImportHook))
+
+
+class NoRequestsSpider(scrapy.Spider):
+    name = "no_request"
+
+    async def start(self):
+        self.logger.info(f"Hooks during run: {hook_count()}")
+        return
+        yield
+
+
+for _ in range(2):
+    process = AsyncCrawlerProcess(settings={"TWISTED_REACTOR_ENABLED": False})
+    process.crawl(NoRequestsSpider)
+    process.start()
+
+logger.info(f"Hooks after runs: {hook_count()}")

tests/AsyncCrawlerProcess/reactorless_import_hook_uninstall.py

@@ -0,0 +1,29 @@
+import logging
+import sys
+
+import scrapy
+from scrapy.crawler import AsyncCrawlerProcess
+from scrapy.utils.reactorless import ReactorImportHook
+
+logger = logging.getLogger(__name__)
+
+
+class NoRequestsSpider(scrapy.Spider):
+    name = "no_request"
+
+    async def start(self):
+        return
+        yield
+
+
+process = AsyncCrawlerProcess(settings={"TWISTED_REACTOR_ENABLED": False})
+
+process.crawl(NoRequestsSpider)
+process.start()
+
+hook_count = sum(1 for finder in sys.meta_path if isinstance(finder, ReactorImportHook))
+logger.info(f"Hooks in sys.meta_path after start(): {hook_count}")
+
+import twisted.internet.reactor  # noqa: E402,F401,TID253
+
+logger.info("Reactor imported after start()")

tests/AsyncCrawlerProcess/reactorless_then_reactor.py

@@ -0,0 +1,19 @@
+import scrapy
+from scrapy.crawler import AsyncCrawlerProcess
+
+
+class NoRequestsSpider(scrapy.Spider):
+    name = "no_request"
+
+    async def start(self):
+        return
+        yield
+
+
+process = AsyncCrawlerProcess(settings={"TWISTED_REACTOR_ENABLED": False})
+process.crawl(NoRequestsSpider)
+process.start()
+
+process2 = AsyncCrawlerProcess()
+process2.crawl(NoRequestsSpider)
+process2.start()

tests/test_crawler_subprocess.py

@@ -407,6 +407,38 @@ class TestAsyncCrawlerProcessSubprocess(TestCrawlerProcessSubprocessBase):
         assert "Spider closed (finished)" in log
         assert "ImportError: Import of twisted.internet.reactor is forbidden" in log
 
+    def test_reactorless_import_hook_uninstall(self) -> None:
+        """The import hook is removed when start() returns, so importing
+        twisted.internet.reactor becomes possible again."""
+        log = self.run_script("reactorless_import_hook_uninstall.py")
+        assert "Not using a Twisted reactor" in log
+        assert "Spider closed (finished)" in log
+        assert "Hooks in sys.meta_path after start(): 0" in log
+        assert "Reactor imported after start()" in log
+        assert "ImportError" not in log
+
+    def test_reactorless_import_hook_multiple(self) -> None:
+        """Sequential AsyncCrawlerProcess instances don't accumulate import
+        hooks: there is exactly one during each run and none afterwards."""
+        log = self.run_script("reactorless_import_hook_multiple.py")
+        assert log.count("Spider closed (finished)") == 2
+        assert log.count("Hooks during run: 1") == 2
+        assert "Hooks after runs: 0" in log
+        assert "ERROR: " not in log
+
+    def test_reactorless_then_reactor(self) -> None:
+        """After a reactorless run finishes, a reactor-based run is possible
+        in the same process."""
+        log = self.run_script("reactorless_then_reactor.py")
+        assert log.count("Spider closed (finished)") == 2
+        assert "Not using a Twisted reactor" in log
+        assert (
+            "Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor"
+            in log
+        )
+        assert "ImportError" not in log
+        assert "ERROR: " not in log
+
     def test_reactorless_telnetconsole_default(self) -> None:
         """By default TWISTED_REACTOR_ENABLED=False silently sets TELNETCONSOLE_ENABLED=False."""
         log = self.run_script("reactorless_simple.py")  # no need for a separate script

tests/test_utils_reactorless.py

@@ -0,0 +1,50 @@
+from __future__ import annotations
+
+import sys
+
+import pytest
+
+from scrapy.utils.reactorless import (
+    ReactorImportHook,
+    install_reactor_import_hook,
+    uninstall_reactor_import_hook,
+)
+
+
+class TestReactorImportHook:
+    def test_install_and_uninstall(self) -> None:
+        hook = install_reactor_import_hook()
+        try:
+            assert isinstance(hook, ReactorImportHook)
+            assert sys.meta_path[0] is hook
+        finally:
+            uninstall_reactor_import_hook(hook)
+        assert hook not in sys.meta_path
+
+    def test_uninstall_twice(self) -> None:
+        hook = install_reactor_import_hook()
+        uninstall_reactor_import_hook(hook)
+        uninstall_reactor_import_hook(hook)
+        assert hook not in sys.meta_path
+
+    def test_uninstall_not_installed(self) -> None:
+        uninstall_reactor_import_hook(ReactorImportHook())
+
+    def test_uninstall_only_removes_the_given_hook(self) -> None:
+        hook1 = install_reactor_import_hook()
+        hook2 = install_reactor_import_hook()
+        try:
+            uninstall_reactor_import_hook(hook1)
+            assert hook1 not in sys.meta_path
+            assert hook2 in sys.meta_path
+        finally:
+            uninstall_reactor_import_hook(hook1)
+            uninstall_reactor_import_hook(hook2)
+
+    def test_find_spec(self) -> None:
+        hook = ReactorImportHook()
+        with pytest.raises(
+            ImportError, match=r"Import of twisted\.internet\.reactor is forbidden"
+        ):
+            hook.find_spec("twisted.internet.reactor", None)
+        assert hook.find_spec("twisted.internet.defer", None) is None