diff --git a/README.CHANGES b/README.CHANGES index bb6de8747a4..e4c34adc408 100644 --- a/README.CHANGES +++ b/README.CHANGES @@ -10,6 +10,11 @@ This covers any developer-visible changes such as a new module API, but e.g. code cleanups which don't have any externally-visible effect do not need to be documented in CHANGES. +CHANGES records what changed between releases, so a bug which was both +introduced and fixed since the last release needs no entry. trunk is +never released, so fixing a regression which only ever existed on trunk +needs none at all. + Changes should be documented by creating a file in changes-entries/ with the .txt suffix, using the following template: @@ -21,6 +26,9 @@ with the .txt suffix, using the following template: Changes to server/*.[ch] use a "core:" prefix rather than "mod_foo:". +Changes to an MPM are named for the MPM rather than the module which +implements it, e.g. "prefork MPM:" rather than "mpm_prefork:". + The description should be as concise as possible, a maximum of three lines but ideally one or two; describe the user-visible effect of the change rather than simply describing how the code was fixed. New diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c index 3de062f367a..37dc2dda4dd 100644 --- a/server/mpm/prefork/prefork.c +++ b/server/mpm/prefork/prefork.c @@ -970,7 +970,11 @@ static int prefork_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s) } } else { - /* Kill 'em off */ + /* Kill 'em off. The parent is in this process group too, and + * its handler would take the signal as a fresh restart request + * and loop; ap_unixd_mpm_set_signals() below puts the handler + * back. */ + apr_signal(SIGHUP, SIG_IGN); if (ap_unixd_killpg(getpgrp(), SIGHUP) < 0) { ap_log_error(APLOG_MARK, APLOG_WARNING, errno, ap_server_conf, APLOGNO(00172) "killpg SIGHUP"); diff --git a/test/modules/arch/linux/env.py b/test/modules/arch/linux/env.py index 96c2bbb2cbd..0e3b4f6dd9d 100644 --- a/test/modules/arch/linux/env.py +++ b/test/modules/arch/linux/env.py @@ -8,7 +8,6 @@ import subprocess import threading import time -from datetime import timedelta from typing import Callable, Dict, List, Optional from pyhttpd.env import HttpdTestEnv, HttpdTestSetup @@ -81,25 +80,6 @@ def server_env(self) -> Dict[str, str]: def httpd_bin(self) -> str: return os.path.join(self.bin_dir, 'httpd') - def apache_hard_restart(self) -> int: - """Restart without the "graceful" flag, so the MPM starts over.""" - r = self._run_apachectl("restart") - if r.exit_code == 0: - return 0 if self.is_live(self._http_base, timeout=timedelta(seconds=10)) else -1 - return r.exit_code - - def read_pid_file(self, name: str = 'httpd.pid') -> Optional[int]: - # Where PidFile lands depends on how the httpd under test resolves - # a relative path against DefaultRuntimeDir, which has differed - # between versions; look in both places rather than assume. - for d in (self.server_logs_dir, self.server_dir): - try: - with open(os.path.join(d, name)) as fd: - return int(fd.read().strip()) - except (OSError, ValueError): - continue - return None - class NotifyListener: """A stand-in for the systemd notification socket. diff --git a/test/modules/core/test_008_restart.py b/test/modules/core/test_008_restart.py new file mode 100644 index 00000000000..7843053477a --- /dev/null +++ b/test/modules/core/test_008_restart.py @@ -0,0 +1,54 @@ +import pytest + +from pyhttpd.conf import HttpdConf + + +class TestRestart: + """The server goes on serving across a restart, graceful or not. + + Whatever the MPM does with its children, what has to hold afterwards + is that a request is answered. A restart which leaves the listening + socket open with nothing behind it is the failure this catches, and + it is not visible from the exit status of "httpd -k restart". + """ + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env): + conf = HttpdConf(env) + conf.add_vhost_test1() + conf.install() + # Restarting is rough on mod_cgid, which is loaded for this + # package: the daemon is signalled along with the rest of the + # process group (AH01239), and restarting again before it has + # unlinked its socket leaves the new one unable to bind + # (AH01243). Neither is what these tests are about. + env.httpd_error_log.add_ignored_lognos(['AH01239', 'AH01243']) + assert env.apache_restart() == 0 + + def get(self, env, when): + # --max-time, or a server which accepts the connection and then + # says nothing hangs here rather than failing. + r = env.curl_get(env.mkurl("http", "test1", "/"), + options=['--max-time', '10']) + assert r.exit_code == 0, f"no answer {when}: {r.stderr}" + assert r.response['status'] == 200, f"bad status {when}" + + def test_core_008_01_graceful(self, env): + self.get(env, "before the reload") + assert env.apache_reload() == 0 + self.get(env, "after a graceful reload") + + def test_core_008_02_ungraceful(self, env): + """"httpd -k restart" keeps the same parent and serves again.""" + self.get(env, "before the restart") + pid = env.read_pid_file() + assert pid, "no pid file" + assert env.apache_hard_restart() == 0 + assert env.read_pid_file() == pid, "the restart replaced the parent" + self.get(env, "after an ungraceful restart") + + def test_core_008_03_repeated(self, env): + """Restarting twice in a row is no different from once.""" + for _ in range(2): + assert env.apache_hard_restart() == 0 + self.get(env, "after repeated restarts") diff --git a/test/pyhttpd/env.py b/test/pyhttpd/env.py index f2f3527e0ce..cba8cd4eda9 100644 --- a/test/pyhttpd/env.py +++ b/test/pyhttpd/env.py @@ -848,6 +848,26 @@ def apache_fail(self): rv = 0 return rv + def apache_hard_restart(self) -> int: + """Restart without the "graceful" flag, so the MPM starts over.""" + r = self._run_apachectl("restart") + if r.exit_code == 0: + return 0 if self.is_live(self._http_base, + timeout=timedelta(seconds=10)) else -1 + return r.exit_code + + def read_pid_file(self, name: str = 'httpd.pid') -> Optional[int]: + # Where PidFile lands depends on how the httpd under test resolves + # a relative path against DefaultRuntimeDir, which has differed + # between versions; look in both places rather than assume. + for d in (self._server_logs_dir, self._server_dir): + try: + with open(os.path.join(d, name)) as fd: + return int(fd.read().strip()) + except (OSError, ValueError): + continue + return None + def apache_access_log_clear(self): if os.path.isfile(self._server_access_log): os.remove(self._server_access_log)