Skip to content

Commit 1bed1a3

Browse files
dunglasclaude
andcommitted
fix: prevent worker starvation after max_execution_time restarts
Disable the execution timer in frankenphp_worker_request_shutdown() to prevent stale timers from firing between requests. Also fix evaluation order in frankenphp_handle_request() to check for shutdown before calling frankenphp_worker_request_startup(). Fixes #2205 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b02d99a commit 1bed1a3

3 files changed

Lines changed: 63 additions & 0 deletions

File tree

frankenphp.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,12 @@ static void frankenphp_cleanup_worker_state(void) {
385385

386386
/* Adapted from php_request_shutdown */
387387
static void frankenphp_worker_request_shutdown() {
388+
#ifdef ZEND_MAX_EXECUTION_TIMERS
389+
/* Disable any execution timer set during the request callback to prevent
390+
* stale timers from firing between requests */
391+
zend_unset_timeout();
392+
#endif
393+
388394
/* Flush all output buffers */
389395
zend_try { php_output_end_all(); }
390396
zend_end_try();
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
require_once __DIR__.'/_executor.php';
4+
5+
return function () {
6+
$action = $_GET['action'] ?? 'normal';
7+
8+
switch ($action) {
9+
case 'timeout':
10+
echo 'BEFORE_TIMEOUT';
11+
set_time_limit(1);
12+
while (true) {
13+
// Infinite loop: will be killed by max_execution_time
14+
}
15+
break;
16+
17+
case 'ping':
18+
echo 'pong';
19+
break;
20+
}
21+
};

worker_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strconv"
1313
"strings"
1414
"testing"
15+
"time"
1516

1617
"github.com/stretchr/testify/require"
1718

@@ -174,3 +175,38 @@ func TestKeepRunningOnConnectionAbort(t *testing.T) {
174175
assert.Equal(t, "requests:2", body2, "should not have stopped execution after the first request was aborted")
175176
}, &testOptions{workerScript: "worker-with-counter.php", nbWorkers: 1, nbParallelRequests: 1})
176177
}
178+
179+
func TestWorkerRecoverFromTimeout(t *testing.T) {
180+
if !frankenphp.Config().ZendMaxExecutionTimers {
181+
t.Skip("requires ZEND_MAX_EXECUTION_TIMERS")
182+
}
183+
184+
runTest(t, func(_ func(http.ResponseWriter, *http.Request), ts *httptest.Server, i int) {
185+
client := &http.Client{Timeout: 5 * time.Second}
186+
187+
// Trigger a request that will hit max_execution_time and crash the worker
188+
resp, err := client.Get(ts.URL + "/worker-timeout-recovery.php?action=timeout")
189+
if err == nil {
190+
_, _ = io.ReadAll(resp.Body)
191+
_ = resp.Body.Close()
192+
}
193+
194+
// After the worker restarts, it should be able to serve new requests
195+
assert.Eventually(t, func() bool {
196+
resp, err := client.Get(ts.URL + "/worker-timeout-recovery.php?action=ping")
197+
if err != nil {
198+
return false
199+
}
200+
201+
body, _ := io.ReadAll(resp.Body)
202+
_ = resp.Body.Close()
203+
204+
return string(body) == "pong"
205+
}, 5*time.Second, 50*time.Millisecond, "worker should recover after max_execution_time timeout")
206+
}, &testOptions{
207+
workerScript: "worker-timeout-recovery.php",
208+
nbWorkers: 1,
209+
nbParallelRequests: 1,
210+
realServer: true,
211+
})
212+
}

0 commit comments

Comments
 (0)