Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions frankenphp.c
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,12 @@ static void frankenphp_cleanup_worker_state(void) {

/* Adapted from php_request_shutdown */
static void frankenphp_worker_request_shutdown() {
#ifdef ZEND_MAX_EXECUTION_TIMERS
/* Disable any execution timer set during the request callback to prevent
* stale timers from firing between requests */
zend_unset_timeout();
#endif

Comment on lines +388 to +393
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this does anything. The timeout can only happen between 2 calls to frankenphp_handle_request(), where it should be save to timeout.

/* Flush all output buffers */
zend_try { php_output_end_all(); }
zend_end_try();
Expand Down
21 changes: 21 additions & 0 deletions testdata/worker-timeout-recovery.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

require_once __DIR__.'/_executor.php';

return function () {
$action = $_GET['action'] ?? 'normal';

switch ($action) {
case 'timeout':
echo 'BEFORE_TIMEOUT';
set_time_limit(1);
while (true) {
// Infinite loop: will be killed by max_execution_time
}
break;

case 'ping':
echo 'pong';
break;
}
};
36 changes: 36 additions & 0 deletions worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"testing"
"time"

"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -174,3 +175,38 @@ func TestKeepRunningOnConnectionAbort(t *testing.T) {
assert.Equal(t, "requests:2", body2, "should not have stopped execution after the first request was aborted")
}, &testOptions{workerScript: "worker-with-counter.php", nbWorkers: 1, nbParallelRequests: 1})
}

func TestWorkerRecoverFromTimeout(t *testing.T) {
if !frankenphp.Config().ZendMaxExecutionTimers {
t.Skip("requires ZEND_MAX_EXECUTION_TIMERS")
}

runTest(t, func(_ func(http.ResponseWriter, *http.Request), ts *httptest.Server, i int) {
client := &http.Client{Timeout: 5 * time.Second}

// Trigger a request that will hit max_execution_time and crash the worker
resp, err := client.Get(ts.URL + "/worker-timeout-recovery.php?action=timeout")
if err == nil {
_, _ = io.ReadAll(resp.Body)
_ = resp.Body.Close()
}

// After the worker restarts, it should be able to serve new requests
assert.Eventually(t, func() bool {
resp, err := client.Get(ts.URL + "/worker-timeout-recovery.php?action=ping")
if err != nil {
return false
}

body, _ := io.ReadAll(resp.Body)
_ = resp.Body.Close()

return string(body) == "pong"
}, 5*time.Second, 50*time.Millisecond, "worker should recover after max_execution_time timeout")
}, &testOptions{
workerScript: "worker-timeout-recovery.php",
nbWorkers: 1,
nbParallelRequests: 1,
realServer: true,
})
}
Loading