diff --git a/tools/run-dev.py b/tools/run-dev.py index 981aa46ca5..1975a52b00 100755 --- a/tools/run-dev.py +++ b/tools/run-dev.py @@ -84,8 +84,10 @@ if options.test: settings_module = "zproject.test_settings" # Don't auto-reload when running Puppeteer tests runserver_args = ["--noreload"] + tornado_autoreload = [] else: settings_module = "zproject.settings" + tornado_autoreload = ["-m", "tornado.autoreload"] manage_args = [f"--settings={settings_module}"] os.environ["DJANGO_SETTINGS_MODULE"] = settings_module @@ -134,6 +136,8 @@ def server_processes() -> List[List[str]]: [ "env", "PYTHONUNBUFFERED=1", + "python3", + *tornado_autoreload, "./manage.py", "runtornado", *manage_args, diff --git a/tools/test-backend b/tools/test-backend index f10cc87e00..fb10db2041 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -115,7 +115,6 @@ not_yet_fully_covered = [ "zerver/lib/test_console_output.py", "zerver/openapi/python_examples.py", # Tornado should ideally have full coverage, but we're not there. - "zerver/tornado/autoreload.py", "zerver/tornado/descriptors.py", "zerver/tornado/django_api.py", "zerver/tornado/event_queue.py", diff --git a/zerver/management/commands/runtornado.py b/zerver/management/commands/runtornado.py index dcc81da3db..935efe778e 100644 --- a/zerver/management/commands/runtornado.py +++ b/zerver/management/commands/runtornado.py @@ -3,9 +3,10 @@ import sys from typing import Any, Callable from urllib.parse import SplitResult +import __main__ from django.conf import settings from django.core.management.base import BaseCommand, CommandError, CommandParser -from tornado import ioloop +from tornado import autoreload, ioloop from tornado.log import app_log # We must call zerver.tornado.ioloop_logging.instrument_tornado_ioloop @@ -20,7 +21,6 @@ instrument_tornado_ioloop() from zerver.lib.debug import interactive_debug_listen from zerver.tornado.application import create_tornado_application, setup_tornado_rabbitmq -from zerver.tornado.autoreload import start as zulip_autoreload_start from zerver.tornado.event_queue import ( add_client_gc_hook, get_wrapped_process_notification, @@ -93,8 +93,6 @@ class Command(BaseCommand): try: # Application is an instance of Django's standard wsgi handler. application = create_tornado_application() - if settings.AUTORELOAD: - zulip_autoreload_start() # start tornado web server in single-threaded mode http_server = httpserver.HTTPServer(application, xheaders=True) @@ -112,8 +110,17 @@ class Command(BaseCommand): if django.conf.settings.DEBUG: instance.set_blocking_log_threshold(5) instance.handle_callback_exception = handle_callback_exception + + if hasattr(__main__, "add_reload_hook"): + autoreload.start() + instance.start() except KeyboardInterrupt: + # Monkey patch tornado.autoreload to prevent it from continuing + # to watch for changes after catching our SystemExit. Otherwise + # the user needs to press Ctrl+C twice. + __main__.wait = lambda: None + sys.exit(0) inner_run() diff --git a/zerver/tornado/application.py b/zerver/tornado/application.py index e77840e1d9..25fdb64596 100644 --- a/zerver/tornado/application.py +++ b/zerver/tornado/application.py @@ -2,9 +2,9 @@ import atexit import tornado.web from django.conf import settings +from tornado import autoreload from zerver.lib.queue import get_queue_client -from zerver.tornado import autoreload from zerver.tornado.handlers import AsyncDjangoHandler diff --git a/zerver/tornado/autoreload.py b/zerver/tornado/autoreload.py deleted file mode 100644 index 645032e6ec..0000000000 --- a/zerver/tornado/autoreload.py +++ /dev/null @@ -1,240 +0,0 @@ -# mypy: ignore_errors - -# Copyright 2009 Facebook -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -"""Automatically restart the server when a source file is modified. - -Most applications should not access this module directly. Instead, -pass the keyword argument ``autoreload=True`` to the -`tornado.web.Application` constructor (or ``debug=True``, which -enables this setting and several others). This will enable autoreload -mode as well as checking for changes to templates and static -resources. Note that restarting is a destructive operation and any -requests in progress will be aborted when the process restarts. (If -you want to disable autoreload while using other debug-mode features, -pass both ``debug=True`` and ``autoreload=False``). - -This module can also be used as a command-line wrapper around scripts -such as unit test runners. See the `main` method for details. - -The command-line wrapper and Application debug modes can be used together. -This combination is encouraged as the wrapper catches syntax errors and -other import-time failures, while debug mode catches changes once -the server has started. - -This module depends on `.IOLoop`, so it will not work in WSGI applications -and Google App Engine. It also will not work correctly when `.HTTPServer`'s -multi-process mode is used. - -Reloading loses any Python interpreter command-line arguments (e.g. ``-u``) -because it re-executes Python using ``sys.executable`` and ``sys.argv``. -Additionally, modifying these variables will cause reloading to behave -incorrectly. - -""" - -# Further patched by Zulip check whether the code we're about to -# reload actually imports before reloading into it. This fixes a -# major development workflow problem, where if one did a `git rebase`, -# Tornado would crash itself by auto-reloading into a version of the -# code that didn't work. - - -import functools -import importlib -import os -import subprocess -import sys -import traceback -import types -import weakref - -from tornado import ioloop, process -from tornado.log import gen_log - -try: - import signal -except ImportError: - signal = None - -# os.execv is broken on Windows and can't properly parse command line -# arguments and executable name if they contain whitespaces. subprocess -# fixes that behavior. -_has_execv = sys.platform != "win32" - -_watched_files = set() -_reload_hooks = [] -_reload_attempted = False -_io_loops = weakref.WeakKeyDictionary() -needs_to_reload = False - - -def start(io_loop=None, check_time=500): - """Begins watching source files for changes. - - .. versionchanged:: 4.1 - The ``io_loop`` argument is deprecated. - """ - io_loop = io_loop or ioloop.IOLoop.current() - if io_loop in _io_loops: - return - _io_loops[io_loop] = True - if len(_io_loops) > 1: - gen_log.warning("tornado.autoreload started more than once in the same process") - modify_times = {} - callback = functools.partial(_reload_on_update, modify_times) - scheduler = ioloop.PeriodicCallback(callback, check_time, io_loop=io_loop) - scheduler.start() - - -def wait(): - """Wait for a watched file to change, then restart the process. - - Intended to be used at the end of scripts like unit test runners, - to run the tests again after any source file changes (but see also - the command-line interface in `main`) - """ - io_loop = ioloop.IOLoop() - start(io_loop) - io_loop.start() - - -def watch(filename): - """Add a file to the watch list. - - All imported modules are watched by default. - """ - _watched_files.add(filename) - - -def add_reload_hook(fn): - """Add a function to be called before reloading the process. - - Note that for open file and socket handles it is generally - preferable to set the ``FD_CLOEXEC`` flag (using `fcntl` or - ``tornado.platform.auto.set_close_exec``) instead - of using a reload hook to close them. - """ - _reload_hooks.append(fn) - - -def _reload_on_update(modify_times): - global needs_to_reload - if _reload_attempted: - # We already tried to reload and it didn't work, so don't try again. - return - if process.task_id() is not None: - # We're in a child process created by fork_processes. If child - # processes restarted themselves, they'd all restart and then - # all call fork_processes again. - return - for module in list(sys.modules.values()): - # Some modules play games with sys.modules (e.g. email/__init__.py - # in the standard library), and occasionally this can cause strange - # failures in getattr. Just ignore anything that's not an ordinary - # module. - if not isinstance(module, types.ModuleType): - continue - path = getattr(module, "__file__", None) - if not path: - continue - if path.endswith(".pyc") or path.endswith(".pyo"): - path = path[:-1] - - result = _check_file(modify_times, module, path) - if result is False: - # If any files errored, we abort this attempt at reloading. - return - if result is True: - # If any files had actual changes that import properly, - # we'll plan to reload the next time we run with no files - # erroring. - needs_to_reload = True - - if needs_to_reload: - _reload() - - -def _check_file(modify_times, module, path): - try: - modified = os.stat(path).st_mtime - except Exception: - return - if path not in modify_times: - modify_times[path] = modified - return - if modify_times[path] != modified: - gen_log.info("%s modified; restarting server", path) - modify_times[path] = modified - else: - return - - if path == __file__ or path == os.path.join(os.path.dirname(__file__), "event_queue.py"): - # Assume that the autoreload library itself imports correctly, - # because reloading this file will destroy its state, - # including _reload_hooks - return True - - try: - importlib.reload(module) - except Exception: - gen_log.error(f"Error importing {path}, not reloading") - traceback.print_exc() - return False - return True - - -def _reload(): - global _reload_attempted - _reload_attempted = True - for fn in _reload_hooks: - fn() - # Make sure any output from reload hooks makes it to stdout. - sys.stdout.flush() - if hasattr(signal, "setitimer"): - # Clear the alarm signal set by - # ioloop.set_blocking_log_threshold so it doesn't fire - # after the exec. - signal.setitimer(signal.ITIMER_REAL, 0, 0) - # sys.path fixes: see comments at top of file. If sys.path[0] is an empty - # string, we were (probably) invoked with -m and the effective path - # is about to change on re-exec. Add the current directory to $PYTHONPATH - # to ensure that the new process sees the same path we did. - path_prefix = "." + os.pathsep - if sys.path[0] == "" and not os.environ.get("PYTHONPATH", "").startswith(path_prefix): - os.environ["PYTHONPATH"] = path_prefix + os.environ.get("PYTHONPATH", "") - if not _has_execv: - subprocess.Popen([sys.executable] + sys.argv) - sys.exit(0) - else: - try: - os.execv(sys.executable, [sys.executable] + sys.argv) - except OSError: - # Mac OS X versions prior to 10.6 do not support execv in - # a process that contains multiple threads. Instead of - # re-executing in the current process, start a new one - # and cause the current process to exit. This isn't - # ideal since the new process is detached from the parent - # terminal and thus cannot easily be killed with Ctrl-C, - # but it's better than not being able to autoreload at - # all. - # Unfortunately the errno returned in this case does not - # appear to be consistent, so we can't easily check for - # this error specifically. - os.spawnv(os.P_NOWAIT, sys.executable, [sys.executable] + sys.argv) - # At this point the IOLoop has been closed and finally - # blocks will experience errors if we allow the stack to - # unwind, so just exit uncleanly. - os._exit(0) diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 860ac5874d..fe794177b9 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -37,6 +37,7 @@ import orjson import tornado.ioloop from django.conf import settings from django.utils.translation import gettext as _ +from tornado import autoreload from version import API_FEATURE_LEVEL, ZULIP_MERGE_BASE, ZULIP_VERSION from zerver.lib.exceptions import JsonableError @@ -46,7 +47,6 @@ from zerver.lib.notification_data import UserMessageNotificationsData from zerver.lib.queue import queue_json_publish, retry_event from zerver.lib.utils import statsd from zerver.middleware import async_request_timer_restart -from zerver.tornado.autoreload import add_reload_hook from zerver.tornado.descriptors import clear_descriptor_by_handler_id, set_descriptor_by_handler_id from zerver.tornado.exceptions import BadEventQueueIdError from zerver.tornado.handlers import ( @@ -622,7 +622,7 @@ def setup_event_queue(server: tornado.httpserver.HTTPServer, port: int) -> None: signal.SIGTERM, lambda signum, frame: ioloop.add_callback_from_signal(handle_sigterm, server), ) - add_reload_hook(lambda: dump_event_queues(port)) + autoreload.add_reload_hook(lambda: dump_event_queues(port)) try: os.rename(persistent_queue_filename(port), persistent_queue_filename(port, last=True)) diff --git a/zproject/computed_settings.py b/zproject/computed_settings.py index f8e2f780f6..60f372dc81 100644 --- a/zproject/computed_settings.py +++ b/zproject/computed_settings.py @@ -232,7 +232,6 @@ if not TORNADO_PORTS: TORNADO_PROCESSES = len(TORNADO_PORTS) RUNNING_INSIDE_TORNADO = False -AUTORELOAD = DEBUG SILENCED_SYSTEM_CHECKS = [ # auth.W004 checks that the UserProfile field named by USERNAME_FIELD has diff --git a/zproject/test_extra_settings.py b/zproject/test_extra_settings.py index 77c0cc2c73..9452d51e00 100644 --- a/zproject/test_extra_settings.py +++ b/zproject/test_extra_settings.py @@ -113,9 +113,6 @@ else: WEBPACK_STATS_FILE = os.path.join(DEPLOY_ROOT, "var", "webpack-stats-test.json") WEBPACK_BUNDLES = "webpack-bundles/" -# Don't auto-restart Tornado server during automated tests -AUTORELOAD = False - if not PUPPETEER_TESTS: # Use local memory cache for backend tests. CACHES["default"] = {