feat: cross-platform force-kill primitive for stuck PHP threads (#2365)

nicolas-grekas · web-flow · commit 2ac6c419fec9 · 2026-05-04T16:38:32.000+02:00
First step of the split suggested in #2287: land the force-kill infrastructure as a standalone, reviewable primitive independent of background workers. ## Design Each PHP thread, at boot from its own TSRM context, hands a `force_kill_slot` (pointers to its `EG(vm_interrupt)` and `EG(timed_out)` atomic bools, plus `pthread_t` / Windows `HANDLE`) back to Go via `go_frankenphp_store_force_kill_slot`. The slot lives on `phpThread` and is protected by a per-thread `RWMutex` so the zero-and-release path at thread exit cannot race an in-flight kill. From any goroutine, Go passes the slot back to `frankenphp_force_kill_thread`, which stores `true` into both atomic bools (waking the VM at the next opcode boundary, routing through `zend_timeout` -> "Maximum execution time exceeded") and delivers a platform-specific wake-up: - **Linux/FreeBSD**: `pthread_kill(SIGRTMIN+3)` with a no-op handler installed once via `pthread_once`, `SA_ONSTACK`, no `SA_RESTART`. Signal delivery returns any in-flight blocking syscall with `EINTR`. - **Windows**: `CancelSynchronousIo` + `QueueUserAPC` covers alertable I/O and `SleepEx`. Non-alertable `Sleep` (including PHP's `usleep`) stays uninterruptible. - **macOS**: atomic-bool path only; threads stuck in blocking syscalls wait for the syscall to complete naturally. **Reserved signal**: `SIGRTMIN+3`. A PHP script that calls `pcntl_signal(SIGRTMIN+3, ...)` clobbers this. Embedders whose own Go code uses `SIGRTMIN+3` must patch it here. glibc NPTL reserves `SIGRTMIN..SIGRTMIN+2`, so the offset cannot go lower. ## Drain integration `drainWorkerThreads` waits `drainGracePeriod` (30s) for each thread to reach `Yielding`, then arms force-kill on stragglers and **keeps waiting** until they yield. `phpThread.shutdown` does the same. There is no abandon path: if a thread is stuck in a syscall force-kill cannot interrupt (macOS, Windows non-alertable Sleep), the drain blocks until the syscall returns naturally — matching pre-patch behaviour exactly, just typically much faster because force-kill cuts a `sleep(60)` down to milliseconds. Operators that want a harder bound rely on their orchestrator (systemd, k8s, supervisord) to SIGKILL the process. `go_frankenphp_on_thread_shutdown` runs on both the healthy path and the unhealthy-during-Shutdown path so `state.Done` is set even when force-kill bails the thread. Without it, `phpThread.shutdown`'s `WaitFor(state.Done)` would never unblock. ## Testing `TestRestartWorkersForceKillsStuckThread` drives the full path via a marker file so `RestartWorkers` only arms once the worker is proven parked in `sleep()`, then asserts bounded elapsed time and that the post-sleep echo never runs.
diff --git a/frankenphp.c b/frankenphp.c
@@ -98,6 +98,111 @@ static bool is_forked_child = false;
 static void frankenphp_fork_child(void) { is_forked_child = true; }
 #endif
 
+/* Best-effort force-kill for stuck PHP threads.
+ *
+ * Each thread captures &EG(vm_interrupt) / &EG(timed_out) at boot and
+ * hands them to Go via go_frankenphp_store_force_kill_slot. To kill,
+ * Go passes the slot back to frankenphp_force_kill_thread, which stores
+ * true into both bools (the VM bails through zend_timeout() at the next
+ * opcode boundary) and then wakes any in-flight syscall:
+ *   - Linux/FreeBSD: pthread_kill(SIGRTMIN+3) -> EINTR.
+ *   - Windows: CancelSynchronousIo + QueueUserAPC for alertable I/O +
+ *     SleepEx. Non-alertable Sleep (including PHP's usleep) stays stuck.
+ *   - macOS: atomic-bool only; busy loops bail, blocking syscalls don't.
+ *
+ * Reserved signal: SIGRTMIN+3. PHP's pcntl_signal(SIGRTMIN+3, ...)
+ * clobbers it. glibc NPTL reserves SIGRTMIN..SIGRTMIN+2; embedders with
+ * their own Go signal usage may need to patch this constant.
+ *
+ * The slot lives Go-side on phpThread; the C side has no global table.
+ * The signal handler is installed once via pthread_once. */
+#ifdef PHP_WIN32
+static void CALLBACK frankenphp_noop_apc(ULONG_PTR param) { (void)param; }
+#endif
+
+#ifdef FRANKENPHP_HAS_KILL_SIGNAL
+/* No-op: delivery itself is what unblocks the syscall via EINTR. */
+static void frankenphp_kill_signal_handler(int sig) { (void)sig; }
+
+static pthread_once_t kill_signal_handler_installed = PTHREAD_ONCE_INIT;
+/* Set to true only after sigaction() succeeds. force_kill_thread skips
+ * pthread_kill when this is false, so a sigaction failure (invalid
+ * signal number, exhausted handler slots, etc.) can't deliver the
+ * signal with its default action (process termination). */
+static zend_atomic_bool kill_signal_handler_active;
+static void install_kill_signal_handler(void) {
+  /* No SA_RESTART so syscalls return EINTR rather than being restarted.
+   * SA_ONSTACK guards against an accidental process-level delivery to a
+   * Go-managed thread, where Go requires the alternate signal stack. */
+  struct sigaction sa;
+  memset(&sa, 0, sizeof(sa));
+  sa.sa_handler = frankenphp_kill_signal_handler;
+  sigemptyset(&sa.sa_mask);
+  sa.sa_flags = SA_ONSTACK;
+  if (sigaction(FRANKENPHP_KILL_SIGNAL, &sa, NULL) == 0) {
+    zend_atomic_bool_store(&kill_signal_handler_active, true);
+  }
+}
+#endif
+
+/* Must run on the PHP thread itself: EG() resolves to its own TSRM
+ * context and pthread_self() captures the right tid. */
+static void frankenphp_register_thread_for_kill(uintptr_t idx) {
+  force_kill_slot slot;
+  memset(&slot, 0, sizeof(slot));
+  slot.vm_interrupt = &EG(vm_interrupt);
+  slot.timed_out = &EG(timed_out);
+#ifdef FRANKENPHP_HAS_KILL_SIGNAL
+  slot.tid = pthread_self();
+  pthread_once(&kill_signal_handler_installed, install_kill_signal_handler);
+#elif defined(PHP_WIN32)
+  if (!DuplicateHandle(GetCurrentProcess(), GetCurrentThread(),
+                       GetCurrentProcess(), &slot.thread_handle, 0, FALSE,
+                       DUPLICATE_SAME_ACCESS)) {
+    /* On failure, force_kill falls back to atomic-bool only. */
+    slot.thread_handle = NULL;
+  }
+#endif
+  go_frankenphp_store_force_kill_slot(idx, slot);
+}
+
+void frankenphp_force_kill_thread(force_kill_slot slot) {
+  if (slot.vm_interrupt == NULL) {
+    /* Boot aborted before the slot was published. */
+    return;
+  }
+
+  /* Atomic stores first: by the time the thread wakes (signal-driven or
+   * natural) the VM sees them and bails through zend_timeout(). */
+  zend_atomic_bool_store(slot.timed_out, true);
+  zend_atomic_bool_store(slot.vm_interrupt, true);
+
+#ifdef FRANKENPHP_HAS_KILL_SIGNAL
+  /* ESRCH (thread already exited) / EINVAL are both benign here.
+   * Skip if sigaction() failed at install time: delivering an unhandled
+   * SIGRTMIN+3 would terminate the process. */
+  if (zend_atomic_bool_load(&kill_signal_handler_active)) {
+    pthread_kill(slot.tid, FRANKENPHP_KILL_SIGNAL);
+  }
+#elif defined(PHP_WIN32)
+  if (slot.thread_handle != NULL) {
+    CancelSynchronousIo(slot.thread_handle);
+    QueueUserAPC((PAPCFUNC)frankenphp_noop_apc, slot.thread_handle, 0);
+  }
+#endif
+}
+
+/* CloseHandle on Windows; no-op on POSIX. */
+void frankenphp_release_thread_for_kill(force_kill_slot slot) {
+#ifdef PHP_WIN32
+  if (slot.thread_handle != NULL) {
+    CloseHandle(slot.thread_handle);
+  }
+#else
+  (void)slot;
+#endif
+}
+
 void frankenphp_update_local_thread_context(bool is_worker) {
   is_worker_thread = is_worker;
 
@@ -1118,6 +1223,16 @@ static void *php_thread(void *arg) {
   snprintf(thread_name, 16, "php-%" PRIxPTR, thread_index);
   set_thread_name(thread_name);
 
+#ifdef FRANKENPHP_HAS_KILL_SIGNAL
+  /* The spawning Go-managed M may block realtime signals, which the
+   * new pthread inherits. Unblock FRANKENPHP_KILL_SIGNAL here so
+   * force-kill deliveries are not silently dropped. */
+  sigset_t unblock;
+  sigemptyset(&unblock);
+  sigaddset(&unblock, FRANKENPHP_KILL_SIGNAL);
+  pthread_sigmask(SIG_UNBLOCK, &unblock, NULL);
+#endif
+
   /* Initial allocation of all global PHP memory for this thread */
 #ifdef ZTS
   (void)ts_resource(0);
@@ -1126,6 +1241,10 @@ static void *php_thread(void *arg) {
 #endif
 #endif
 
+  /* Publish this thread's force-kill slot to Go so the graceful-drain
+   * grace period can wake it from a busy PHP loop or blocking syscall. */
+  frankenphp_register_thread_for_kill(thread_index);
+
   bool thread_is_healthy = true;
   bool has_attempted_shutdown = false;
 
@@ -1203,6 +1322,11 @@ static void *php_thread(void *arg) {
   }
   zend_end_try();
 
+  /* Must precede ts_free_thread: that frees the TSRM storage backing
+   * the slot's &EG() pointers. Clearing first means any concurrent
+   * force-kill either ran before us or sees a zero slot. */
+  go_frankenphp_clear_force_kill_slot(thread_index);
+
   /* free all global PHP memory reserved for this thread */
 #ifdef ZTS
   ts_free_thread();
@@ -1211,12 +1335,9 @@ static void *php_thread(void *arg) {
   /* Thread is healthy, signal to Go that the thread has shut down */
   if (thread_is_healthy) {
     go_frankenphp_on_thread_shutdown(thread_index);
-
     return NULL;
   }
 
-  /* Thread is unhealthy, PHP globals might be in a bad state after a bailout,
-   * restart the entire thread */
   frankenphp_log_message("Restarting unhealthy thread", LOG_WARNING);
 
   if (!frankenphp_new_php_thread(thread_index)) {
@@ -1318,7 +1439,9 @@ static void *php_main(void *arg) {
 
   go_frankenphp_main_thread_is_ready();
 
-  /* channel closed, shutdown gracefully */
+  /* channel closed, shutdown gracefully. drainPHPThreads has already
+   * waited for every PHP thread to exit (state.Done), so SAPI/TSRM
+   * teardown here is safe. */
   frankenphp_sapi_module.shutdown(&frankenphp_sapi_module);
 
   sapi_shutdown();
diff --git a/frankenphp.h b/frankenphp.h
@@ -46,6 +46,28 @@ static inline HRESULT LongLongSub(LONGLONG llMinuend, LONGLONG llSubtrahend,
 #include <stdbool.h>
 #include <stdint.h>
 
+#ifndef PHP_WIN32
+#include <pthread.h>
+#include <signal.h>
+#endif
+
+/* Platform capabilities for the force-kill primitive; declared in the
+ * header so Go (via CGo) gets the correct struct layout too. */
+#if !defined(PHP_WIN32) && defined(SIGRTMIN)
+#define FRANKENPHP_HAS_KILL_SIGNAL 1
+#define FRANKENPHP_KILL_SIGNAL (SIGRTMIN + 3)
+#endif
+
+typedef struct {
+  zend_atomic_bool *vm_interrupt;
+  zend_atomic_bool *timed_out;
+#ifdef FRANKENPHP_HAS_KILL_SIGNAL
+  pthread_t tid;
+#elif defined(PHP_WIN32)
+  HANDLE thread_handle;
+#endif
+} force_kill_slot;
+
 #ifndef FRANKENPHP_VERSION
 #define FRANKENPHP_VERSION dev
 #endif
@@ -193,6 +215,17 @@ void frankenphp_init_thread_metrics(int max_threads);
 void frankenphp_destroy_thread_metrics(void);
 size_t frankenphp_get_thread_memory_usage(uintptr_t thread_index);
 
+/* Best-effort force-kill primitives. The slot is populated by each PHP
+ * thread at boot (an internal helper calls back into Go via
+ * go_frankenphp_store_force_kill_slot) and lives in the Go-side phpThread.
+ * force_kill_thread interrupts the Zend VM at the next opcode boundary;
+ * on POSIX it also delivers SIGRTMIN+3 to the target thread, on Windows
+ * it calls CancelSynchronousIo + QueueUserAPC. release_thread drops any
+ * OS-owned resource tied to the slot (currently the Windows thread
+ * handle). */
+void frankenphp_force_kill_thread(force_kill_slot slot);
+void frankenphp_release_thread_for_kill(force_kill_slot slot);
+
 void register_extensions(zend_module_entry **m, int len);
 
 #endif
diff --git a/phpmainthread.go b/phpmainthread.go
@@ -54,6 +54,8 @@ func initPHPThreads(numThreads int, numMaxThreads int, phpIni map[string]string)
 		return nil, err
 	}
 
+	// Must follow start(): maxThreads is only final once
+	// setAutomaticMaxThreads runs on the main PHP thread (before Ready).
 	C.frankenphp_init_thread_metrics(C.int(mainThread.maxThreads))
 
 	// initialize all other threads
@@ -79,6 +81,11 @@ func drainPHPThreads() {
 	if mainThread == nil {
 		return // mainThread was never initialized
 	}
+	// Idempotent: post-drain state is Reserved; a re-entry (e.g. a
+	// failed-Init cleanup) must not double-close mainThread.done.
+	if mainThread.state.Is(state.Reserved) {
+		return
+	}
 	doneWG := sync.WaitGroup{}
 	doneWG.Add(len(phpThreads))
 	mainThread.state.Set(state.ShuttingDown)
diff --git a/phpthread.go b/phpthread.go
@@ -8,6 +8,7 @@ import (
 	"runtime"
 	"sync"
 	"sync/atomic"
+	"time"
 	"unsafe"
 
 	"github.com/dunglas/frankenphp/internal/state"
@@ -25,6 +26,12 @@ type phpThread struct {
 	contextMu    sync.RWMutex
 	state        *state.ThreadState
 	requestCount atomic.Int64
+	// forceKill holds &EG() pointers captured on the PHP thread itself.
+	// forceKillMu pairs with go_frankenphp_clear_force_kill_slot's write
+	// lock so a concurrent kill never dereferences pointers freed by
+	// ts_free_thread.
+	forceKillMu sync.RWMutex
+	forceKill   C.force_kill_slot
 }
 
 // threadHandler defines how the callbacks from the C thread should be handled
@@ -99,7 +106,27 @@ func (thread *phpThread) shutdown() {
 	}
 
 	close(thread.drainChan)
-	thread.state.WaitFor(state.Done)
+
+	// Arm force-kill after the grace period to wake any thread stuck in
+	// a blocking syscall (sleep, blocking I/O). The wait remains
+	// unbounded - on platforms where force-kill cannot interrupt the
+	// syscall (macOS, Windows non-alertable Sleep) the thread will exit
+	// when the syscall completes naturally; the operator's orchestrator
+	// is responsible for any harder timeout.
+	done := make(chan struct{})
+	go func() {
+		thread.state.WaitFor(state.Done)
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(drainGracePeriod):
+		thread.forceKillMu.RLock()
+		C.frankenphp_force_kill_thread(thread.forceKill)
+		thread.forceKillMu.RUnlock()
+		<-done
+	}
+
 	thread.drainChan = make(chan struct{})
 
 	// threads go back to the reserved state from which they can be booted again
@@ -209,6 +236,29 @@ func go_frankenphp_after_script_execution(threadIndex C.uintptr_t, exitStatus C.
 	thread.Unpin()
 }
 
+//export go_frankenphp_store_force_kill_slot
+func go_frankenphp_store_force_kill_slot(threadIndex C.uintptr_t, slot C.force_kill_slot) {
+	thread := phpThreads[threadIndex]
+	thread.forceKillMu.Lock()
+	// Release any prior slot's OS resource (Windows HANDLE) before
+	// overwriting; a phpThread can reboot and re-register.
+	C.frankenphp_release_thread_for_kill(thread.forceKill)
+	thread.forceKill = slot
+	thread.forceKillMu.Unlock()
+}
+
+//export go_frankenphp_clear_force_kill_slot
+func go_frankenphp_clear_force_kill_slot(threadIndex C.uintptr_t) {
+	// Called from C before ts_free_thread on both exit paths. Zeroing
+	// the slot under the write lock guarantees any concurrent kill
+	// either completed before we got the lock or sees a zero slot.
+	thread := phpThreads[threadIndex]
+	thread.forceKillMu.Lock()
+	C.frankenphp_release_thread_for_kill(thread.forceKill)
+	thread.forceKill = C.force_kill_slot{}
+	thread.forceKillMu.Unlock()
+}
+
 //export go_frankenphp_on_thread_shutdown
 func go_frankenphp_on_thread_shutdown(threadIndex C.uintptr_t) {
 	thread := phpThreads[threadIndex]
diff --git a/testdata/worker-sleep.php b/testdata/worker-sleep.php
@@ -0,0 +1,21 @@
+<?php
+
+// Worker that sleeps inside the handler to simulate a stuck request blocking
+// drain. Used to test the force-kill grace period.
+//
+// Before sleeping we touch a marker file whose path is passed via the
+// SLEEP_MARKER header. The Go test polls for the file so it only arms
+// RestartWorkers once the worker is proven to be inside sleep(), removing
+// the fixed-time race of a bare time.Sleep on the caller side.
+$fn = static function () {
+    $marker = $_SERVER['HTTP_SLEEP_MARKER'] ?? '';
+    if ($marker !== '') {
+        touch($marker);
+    }
+    sleep(60);
+    echo 'should not reach';
+};
+
+do {
+    $ret = \frankenphp_handle_request($fn);
+} while ($ret);
diff --git a/worker.go b/worker.go
diff --git a/worker_internal_test.go b/worker_internal_test.go
