diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 22fd65be7..6f54e62e9 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -404,8 +404,8 @@ class CrawlerProcess(CrawlerRunner): :param bool stop_after_crawl: stop or not the reactor when all crawlers have finished - :param bool install_signal_handlers: whether to install the shutdown - handlers (default: True) + :param bool install_signal_handlers: whether to install the OS signal + handlers from Twisted and Scrapy (default: True) """ from twisted.internet import reactor @@ -416,15 +416,17 @@ class CrawlerProcess(CrawlerRunner): return d.addBoth(self._stop_reactor) - if install_signal_handlers: - install_shutdown_handlers(self._signal_shutdown) resolver_class = load_object(self.settings["DNS_RESOLVER"]) resolver = create_instance(resolver_class, self.settings, self, reactor=reactor) resolver.install_on_reactor() tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint("REACTOR_THREADPOOL_MAXSIZE")) reactor.addSystemEventTrigger("before", "shutdown", self.stop) - reactor.run(installSignalHandlers=False) # blocking call + if install_signal_handlers: + reactor.addSystemEventTrigger( + "after", "startup", install_shutdown_handlers, self._signal_shutdown + ) + reactor.run(installSignalHandlers=install_signal_handlers) # blocking call def _graceful_stop_reactor(self) -> Deferred: d = self.stop() diff --git a/scrapy/utils/ossignal.py b/scrapy/utils/ossignal.py index 2334ea792..db9a71273 100644 --- a/scrapy/utils/ossignal.py +++ b/scrapy/utils/ossignal.py @@ -19,13 +19,10 @@ def install_shutdown_handlers( function: SignalHandlerT, override_sigint: bool = True ) -> None: """Install the given function as a signal handler for all common shutdown - signals (such as SIGINT, SIGTERM, etc). If override_sigint is ``False`` the - SIGINT handler won't be install if there is already a handler in place - (e.g. Pdb) + signals (such as SIGINT, SIGTERM, etc). If ``override_sigint`` is ``False`` the + SIGINT handler won't be installed if there is already a handler in place + (e.g. Pdb) """ - from twisted.internet import reactor - - reactor._handleSignals() signal.signal(signal.SIGTERM, function) if signal.getsignal(signal.SIGINT) == signal.default_int_handler or override_sigint: signal.signal(signal.SIGINT, function) diff --git a/scrapy/utils/testproc.py b/scrapy/utils/testproc.py index 5f7a7db14..0688e014b 100644 --- a/scrapy/utils/testproc.py +++ b/scrapy/utils/testproc.py @@ -2,7 +2,7 @@ from __future__ import annotations import os import sys -from typing import Iterable, Optional, Tuple, cast +from typing import Iterable, List, Optional, Tuple, cast from twisted.internet.defer import Deferred from twisted.internet.error import ProcessTerminated @@ -26,14 +26,15 @@ class ProcessTest: env = os.environ.copy() if settings is not None: env["SCRAPY_SETTINGS_MODULE"] = settings + assert self.command cmd = self.prefix + [self.command] + list(args) pp = TestProcessProtocol() - pp.deferred.addBoth(self._process_finished, cmd, check_code) + pp.deferred.addCallback(self._process_finished, cmd, check_code) reactor.spawnProcess(pp, cmd[0], cmd, env=env, path=self.cwd) return pp.deferred def _process_finished( - self, pp: TestProcessProtocol, cmd: str, check_code: bool + self, pp: TestProcessProtocol, cmd: List[str], check_code: bool ) -> Tuple[int, bytes, bytes]: if pp.exitcode and check_code: msg = f"process {cmd} exit with code {pp.exitcode}" diff --git a/setup.py b/setup.py index 47c0af0b0..405633f55 100644 --- a/setup.py +++ b/setup.py @@ -6,8 +6,7 @@ version = (Path(__file__).parent / "scrapy/VERSION").read_text("ascii").strip() install_requires = [ - # 23.8.0 incompatibility: https://github.com/scrapy/scrapy/issues/6024 - "Twisted>=18.9.0,<23.8.0", + "Twisted>=18.9.0", "cryptography>=36.0.0", "cssselect>=0.9.1", "itemloaders>=1.0.1", diff --git a/tests/CrawlerProcess/sleeping.py b/tests/CrawlerProcess/sleeping.py new file mode 100644 index 000000000..420d9d328 --- /dev/null +++ b/tests/CrawlerProcess/sleeping.py @@ -0,0 +1,24 @@ +from twisted.internet.defer import Deferred + +import scrapy +from scrapy.crawler import CrawlerProcess +from scrapy.utils.defer import maybe_deferred_to_future + + +class SleepingSpider(scrapy.Spider): + name = "sleeping" + + start_urls = ["data:,;"] + + async def parse(self, response): + from twisted.internet import reactor + + d = Deferred() + reactor.callLater(3, d.callback, None) + await maybe_deferred_to_future(d) + + +process = CrawlerProcess(settings={}) + +process.crawl(SleepingSpider) +process.start() diff --git a/tests/requirements.txt b/tests/requirements.txt index c07fda2d6..d4bfead40 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,5 +1,6 @@ # Tests requirements attrs +pexpect >= 4.8.0 pyftpdlib >= 1.5.8 pytest pytest-cov==4.0.0 diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 6589381f3..7d87eb62c 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,11 +1,15 @@ +import sys +from io import BytesIO from pathlib import Path +from pexpect.popen_spawn import PopenSpawn from twisted.internet import defer from twisted.trial import unittest from scrapy.utils.testproc import ProcessTest from scrapy.utils.testsite import SiteTest from tests import NON_EXISTING_RESOLVABLE, tests_datadir +from tests.mockserver import MockServer class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @@ -133,3 +137,25 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): args = ["-c", code, "--set", f"TWISTED_REACTOR={reactor_path}"] _, _, err = yield self.execute(args, check_code=True) self.assertNotIn(b"RuntimeError: There is no current event loop in thread", err) + + +class InteractiveShellTest(unittest.TestCase): + def test_fetch(self): + args = ( + sys.executable, + "-m", + "scrapy.cmdline", + "shell", + ) + logfile = BytesIO() + p = PopenSpawn(args, timeout=5) + p.logfile_read = logfile + p.expect_exact("Available Scrapy objects") + with MockServer() as mockserver: + p.sendline(f"fetch('{mockserver.url('/')}')") + p.sendline("type(response)") + p.expect_exact("HtmlResponse") + p.sendeof() + p.wait() + logfile.seek(0) + self.assertNotIn("Traceback", logfile.read().decode()) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 2b141e894..60b92377d 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -1,13 +1,16 @@ import logging import os import platform +import signal import subprocess import sys import warnings from pathlib import Path +from typing import List import pytest from packaging.version import parse as parse_version +from pexpect.popen_spawn import PopenSpawn from pytest import mark, raises from twisted.internet import defer from twisted.trial import unittest @@ -289,9 +292,12 @@ class ScriptRunnerMixin: script_dir: Path cwd = os.getcwd() - def run_script(self, script_name: str, *script_args): + def get_script_args(self, script_name: str, *script_args: str) -> List[str]: script_path = self.script_dir / script_name - args = [sys.executable, str(script_path)] + list(script_args) + return [sys.executable, str(script_path)] + list(script_args) + + def run_script(self, script_name: str, *script_args: str) -> str: + args = self.get_script_args(script_name, *script_args) p = subprocess.Popen( args, env=get_mockserver_env(), @@ -517,6 +523,29 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): self.assertIn("Spider closed (finished)", log) self.assertIn("The value of FOO is 42", log) + def test_shutdown_graceful(self): + sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK + args = self.get_script_args("sleeping.py") + p = PopenSpawn(args, timeout=5) + p.expect_exact("Spider opened") + p.expect_exact("Crawled (200)") + p.kill(sig) + p.expect_exact("shutting down gracefully") + p.expect_exact("Spider closed (shutdown)") + p.wait() + + def test_shutdown_forced(self): + sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK + args = self.get_script_args("sleeping.py") + p = PopenSpawn(args, timeout=5) + p.expect_exact("Spider opened") + p.expect_exact("Crawled (200)") + p.kill(sig) + p.expect_exact("shutting down gracefully") + p.kill(sig) + p.expect_exact("forcing unclean shutdown") + p.wait() + class CrawlerRunnerSubprocess(ScriptRunnerMixin, unittest.TestCase): script_dir = Path(__file__).parent.resolve() / "CrawlerRunner"