Commit bb8461b
committed
feat: cross-platform force-kill primitive for stuck PHP threads
Introduces a self-contained primitive that unblocks a PHP thread stuck
in a blocking call (sleep, synchronous I/O, etc.) so the graceful drain
used by RestartWorkers, DrainWorkers, and Shutdown makes progress
instead of hanging for the duration of the block. The primitive is
useful on its own and gives follow-up graceful-shutdown work a reviewed
foundation to build on.
Design: each PHP thread, at boot from its own TSRM context, hands a
force_kill_slot (pointers to its own EG(vm_interrupt) and EG(timed_out)
atomic bools, plus pthread_t / Windows HANDLE for the wake-up) 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 bools (waking the VM at the next opcode boundary and routing
through zend_timeout -> "Maximum execution time exceeded") and then
delivers a platform-specific wake-up:
- Linux/FreeBSD: pthread_kill(SIGRTMIN+3) with a no-op handler installed
via pthread_once, SA_ONSTACK, no SA_RESTART. Signal delivery causes
any in-flight blocking syscall to return EINTR.
- Windows: CancelSynchronousIo + QueueUserAPC covers alertable I/O and
SleepEx. Non-alertable Sleep (including PHP's usleep) stays
uninterruptible.
- macOS: atomic-bool-only path. Threads stuck in blocking syscalls wait
to return on their own.
JIT caveat: under the OPcache JIT some hot code paths skip vm_interrupt
checks (see php-src#21267), so a pure-PHP busy loop under JIT may not
observe the store and will fall through to the abandon path below.
Drain flow:
- worker.go: drainWorkerThreads waits drainGracePeriod (5s) for each
drained thread to reach Yielding; then arms force-kill on stragglers
and waits forceKillDeadline (5s) more. Threads still stuck past that
are abandoned rather than hanging the drain forever.
- drainWorkerThreads returns (drained, abandoned). RestartWorkers puts
drained threads back to Ready and abandoned ones into the new
state.Abandoned (handlers treat it like ShuttingDown on next
callback) so an abandoned thread that finally unwinds exits instead
of re-entering the serve loop under stale request state. Abandoned
threads are also filtered out of worker.threads immediately so
isAtThreadLimit and the scaler see accurate capacity; the matching
deactivateThreads cleanup drops Abandoned/Done entries from the
auto-scaling slice so they do not permanently consume a global
scaling slot. If any were abandoned, RestartWorkers returns
errIncompleteRestart wrapped with abandoned/restarted counts -
admin endpoint and watcher surface it.
- phpthread.go: phpThread.shutdown mirrors the same grace + force-kill
+ abandon pattern so Shutdown cannot hang on an uninterruptible
blocking call either. RequestSafeStateChange and shutdown's fast-fail
WaitFor both accept Abandoned so Shutdown racing a RestartWorkers
that marks a thread Abandoned does not park on a transition that
will never come.
Lifecycle hardening: Shutdown intentionally leaves phpThreads allocated
and thread_metrics alive - an abandoned thread that eventually unwinds
still calls through the SAPI and lifecycle callbacks which index those
structures. initPHPThreads blocks on a package-level sync.WaitGroup
(Add on every php_thread entry, Done on every exit path, routed
through a single goto exit: label in php_thread so future return paths
cannot silently leak an Add) so the next Init cycle cannot reassign
them out from under a lingering abandoned thread, then frees the
previous allocation inside frankenphp_init_thread_metrics before
allocating fresh. A dedicated C-side atomic (shutdown_in_progress,
toggled by frankenphp_set_shutdown_in_progress) is the signal the
unhealthy-thread restart path uses to refuse respawning past Shutdown.
- go_frankenphp_store_force_kill_slot / clear_force_kill_slot /
on_thread_shutdown: take the per-thread write lock; clear runs before
ts_free_thread on both healthy and unhealthy exit paths so the
captured &EG() pointers are zeroed before their backing storage is
freed.
- php_thread unblocks FRANKENPHP_KILL_SIGNAL with pthread_sigmask at
startup so Go's runtime signal mask cannot silently drop deliveries.
Teardown after abandonment: php_main runs sapi_shutdown / tsrm_shutdown
unconditionally once mainThread.state.Done is observed. By definition
we already gave up on abandoned threads in drainPHPThreads, so we tear
down rather than try to outlive them. If an abandoned thread ever does
unwind after teardown it would touch torn-down state; embedders that
observe errIncompleteRestart and care about cleanliness should
terminate rather than re-Init.
- worker_test.go + testdata/worker-sleep.php: the regression test
drives the full path via a request marker file so it only arms
RestartWorkers once the worker is proven parked in sleep(), then
asserts both the bounded elapsed time and that the "should not
reach" line after sleep never runs (which would indicate the VM
interrupt was never observed).
RestartWorkers now returns an error - a source-compatible Go API change
(callers that ignored it still compile) but worth noting for embedders.1 parent a05e6dd commit bb8461b
16 files changed
Lines changed: 546 additions & 46 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
43 | 49 | | |
44 | 50 | | |
45 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
95 | 196 | | |
96 | 197 | | |
97 | 198 | | |
| |||
1065 | 1166 | | |
1066 | 1167 | | |
1067 | 1168 | | |
| 1169 | + | |
| 1170 | + | |
| 1171 | + | |
| 1172 | + | |
| 1173 | + | |
| 1174 | + | |
| 1175 | + | |
| 1176 | + | |
| 1177 | + | |
| 1178 | + | |
| 1179 | + | |
| 1180 | + | |
| 1181 | + | |
| 1182 | + | |
1068 | 1183 | | |
1069 | 1184 | | |
1070 | 1185 | | |
| |||
1073 | 1188 | | |
1074 | 1189 | | |
1075 | 1190 | | |
| 1191 | + | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
| 1195 | + | |
1076 | 1196 | | |
1077 | 1197 | | |
1078 | 1198 | | |
| |||
1150 | 1270 | | |
1151 | 1271 | | |
1152 | 1272 | | |
| 1273 | + | |
| 1274 | + | |
| 1275 | + | |
| 1276 | + | |
| 1277 | + | |
1153 | 1278 | | |
1154 | 1279 | | |
1155 | 1280 | | |
| |||
1158 | 1283 | | |
1159 | 1284 | | |
1160 | 1285 | | |
1161 | | - | |
1162 | | - | |
| 1286 | + | |
1163 | 1287 | | |
1164 | 1288 | | |
1165 | | - | |
1166 | | - | |
| 1289 | + | |
| 1290 | + | |
| 1291 | + | |
| 1292 | + | |
| 1293 | + | |
| 1294 | + | |
| 1295 | + | |
| 1296 | + | |
1167 | 1297 | | |
1168 | 1298 | | |
1169 | 1299 | | |
1170 | 1300 | | |
1171 | 1301 | | |
1172 | 1302 | | |
1173 | 1303 | | |
| 1304 | + | |
| 1305 | + | |
| 1306 | + | |
| 1307 | + | |
1174 | 1308 | | |
1175 | 1309 | | |
1176 | 1310 | | |
| |||
1265 | 1399 | | |
1266 | 1400 | | |
1267 | 1401 | | |
1268 | | - | |
| 1402 | + | |
| 1403 | + | |
| 1404 | + | |
| 1405 | + | |
| 1406 | + | |
| 1407 | + | |
| 1408 | + | |
| 1409 | + | |
1269 | 1410 | | |
1270 | 1411 | | |
1271 | 1412 | | |
| |||
1470 | 1611 | | |
1471 | 1612 | | |
1472 | 1613 | | |
| 1614 | + | |
| 1615 | + | |
| 1616 | + | |
| 1617 | + | |
1473 | 1618 | | |
1474 | 1619 | | |
1475 | 1620 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
49 | 71 | | |
50 | 72 | | |
51 | 73 | | |
| |||
193 | 215 | | |
194 | 216 | | |
195 | 217 | | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
196 | 231 | | |
197 | 232 | | |
198 | 233 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
38 | 45 | | |
39 | 46 | | |
40 | 47 | | |
| |||
67 | 74 | | |
68 | 75 | | |
69 | 76 | | |
| 77 | + | |
| 78 | + | |
70 | 79 | | |
71 | 80 | | |
72 | 81 | | |
| |||
172 | 181 | | |
173 | 182 | | |
174 | 183 | | |
175 | | - | |
176 | | - | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
177 | 187 | | |
178 | 188 | | |
179 | 189 | | |
180 | | - | |
181 | 190 | | |
182 | 191 | | |
183 | 192 | | |
| |||
187 | 196 | | |
188 | 197 | | |
189 | 198 | | |
190 | | - | |
191 | | - | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
192 | 202 | | |
193 | 203 | | |
194 | 204 | | |
| |||
0 commit comments