fix: avoid fork() deadlock from alarm mutex in helper children

The alarm clock (the runaway-softcode/PCRE CPU limiter) was migrated from
a SIGPROF interval timer to a std::thread + condition_variable, but the
post-fork children still called alarm_clock.clear(), which locks
mux_alarm::mutex_. That mutex is held by the alarm thread on every command
via set()/clear().

fork() duplicates only the calling thread, so the alarm thread does not
exist in the child and the alarm can never fire there. If fork() lands in
the alarm thread's brief mutex window, the child inherits mutex_ locked
with no owner and self-deadlocks in clear() before exec. A later stubslave
reboot/shutdown then blocks the parent forever in waitpid(pid, NULL, 0),
parking the entire network loop: idle, no CPU, not accepting connections,
reconnect gets nothing, and only a full restart clears it. The rare race
plus the need for a subsequent slave reboot explains why it took days.

Fixes:
- Child side (root cause): replace alarm_clock.clear() with the lock-free,
  async-signal-safe alarm_clock.alarmed.store(false) in the three fork
  children (stubslave boot, database dump, helper process).
- Defense in depth: add reap_child_bounded() (WNOHANG poll, then SIGKILL)
  in place of the two blocking waitpid() calls in the stubslave paths, so
  no wedged child can ever park the main thread again. This also cleans up
  the orphaned stubslaves that accumulated across restarts.

The legitimate main-thread alarm_clock.clear() calls in net.cpp and
cque.cpp (which arm/disarm the CPU limiter around command execution) are
unchanged.

Pending field validation on the farm test deployment; do not push yet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stephen Dennis 2026-07-11 16:59:18 +00:00
parent eb93309f6a
commit 0c413ffcaa
4 changed files with 60 additions and 11 deletions

View file

@ -39,6 +39,20 @@ performance work and a large expansion of the smoke-test suite.
## Reliability and Restart
- A `fork()`-with-locked-mutex deadlock that could hang the server after
days of uptime is fixed. The alarm clock (the runaway-softcode/PCRE CPU
limiter) is a `std::thread` plus condition variable whose mutex is taken
briefly on every command. The stubslave, database-dump, and helper-
process children each called `alarm_clock.clear()` immediately after
`fork()`; since `fork()` copies only the calling thread, a child that
forked during the alarm thread's mutex window inherited that mutex locked
with no owner and self-deadlocked in `clear()` before `exec`. A later
stubslave reboot then blocked the parent forever in `waitpid()`, parking
the whole network loop — idle, no CPU, refusing new connections — until a
manual restart. The children now reset the lock-free `alarmed` flag
instead of taking the mutex, and the parent's blocking `waitpid()` calls
are bounded (poll, then `SIGKILL`) so a wedged child can never stall the
server.
- A connect-time crash loop is fixed. A player whose `A_LOGINDATA` was
truncated or malformed made `decrypt_logindata()` dereference a null
field pointer and `SIGSEGV` on connect; and, separately,

View file

@ -1767,10 +1767,12 @@ void fork_and_dump(int key)
}
if (child == 0)
{
// If we don't clear this alarm, the child will eventually receive a
// SIG_PROF.
//
alarm_clock.clear();
// In the forked child the alarm thread does not exist, so the alarm
// can never fire here. Only reset the lock-free, async-signal-safe
// flag so the dump runs unabbreviated. Do NOT call
// alarm_clock.clear() — it locks a std::mutex that may have been
// inherited locked across fork(), deadlocking the dump child.
alarm_clock.alarmed.store(false);
#endif // HAVE_WORKING_FORK
if (key & DUMP_STRUCT)

View file

@ -32,6 +32,7 @@
#if defined(__linux__)
#include <dirent.h> // count_open_fds() reads /proc/self/fd
#endif
#include <csignal> // kill(), SIGKILL — bounded child reaping
#endif
#if defined(_WIN32)
@ -3159,6 +3160,32 @@ extern QUEUE_INFO Queue_In;
extern QUEUE_INFO Queue_Out;
extern pid_t stubslave_pid;
// Reap a forked child without ever blocking the main thread forever. A child
// that wedges before exec would otherwise park us in waitpid() with no timeout,
// stalling the entire network loop (idle, no CPU, not accepting). Poll briefly,
// then force the issue with SIGKILL (guaranteed to terminate a futex-blocked
// process) so a wedged child can never park the server.
static void reap_child_bounded(pid_t pid)
{
if (pid <= 0)
{
return;
}
for (int i = 0; i < 50; i++) // ~5s grace at 100ms
{
pid_t r = waitpid(pid, nullptr, WNOHANG);
if (r == pid || (r == -1 && errno == ECHILD))
{
return;
}
usleep(100000);
}
kill(pid, SIGKILL);
while (waitpid(pid, nullptr, 0) == -1 && errno == EINTR)
{
}
}
bool GanlAdapter::boot_stubslave()
{
const char *pFailedFunc = nullptr;
@ -3205,10 +3232,12 @@ bool GanlAdapter::boot_stubslave()
case 0:
// If we don't clear this alarm, the child will eventually receive a
// SIG_PROF.
//
alarm_clock.clear();
// In the forked child the alarm thread does not exist, so the alarm can
// never fire here. Only reset the lock-free, async-signal-safe flag.
// Do NOT call alarm_clock.clear() — it locks a std::mutex that may have
// been inherited locked across fork(), deadlocking the child before exec
// (and then parking the parent forever in the waitpid() below).
alarm_clock.alarmed.store(false);
// Child. The following calls to dup2() assume only the minimal
// dup2() functionality. That is, the destination descriptor is
@ -3253,7 +3282,7 @@ failure:
if (stubslave_pid > 0)
{
waitpid(stubslave_pid, nullptr, 0);
reap_child_bounded(stubslave_pid);
}
stubslave_pid = 0;
@ -3278,7 +3307,7 @@ void GanlAdapter::shutdown_stubslave()
if (stubslave_pid > 0)
{
waitpid(stubslave_pid, nullptr, 0);
reap_child_bounded(stubslave_pid);
}
stubslave_pid = 0;
}

View file

@ -208,7 +208,11 @@ MUX_RESULT CPlatform::BootHelperProcess(
goto failure;
case 0:
alarm_clock.clear();
// Child: the alarm thread does not exist post-fork, so the alarm can
// never fire here. Reset the lock-free flag only — do NOT call
// alarm_clock.clear(), which locks a std::mutex possibly inherited
// locked across fork() and would deadlock the child before exec.
alarm_clock.alarmed.store(false);
mux_close(sv[0]);
if (sv[1] != 0)
{