diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index 3444772..b5311c7 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -149,6 +149,23 @@ jobs: --set-timeout 120 \ tests/phpt + - name: Collect crash backtraces + if: failure() + run: | + dest="$GITHUB_WORKSPACE/http-server/tests/phpt/crash-reports" + mkdir -p "$dest" + # macOS writes a per-crash report (.ips on 12+, .crash on older) with a + # symbolicated backtrace to DiagnosticReports; grab the recent php ones. + for d in "$HOME/Library/Logs/DiagnosticReports" /Library/Logs/DiagnosticReports; do + [ -d "$d" ] || continue + find "$d" -name 'php-*' -newermt '-30 minutes' 2>/dev/null | while read -r f; do + echo "==================== $f ====================" + cat "$f" + cp "$f" "$dest/" 2>/dev/null || true + done + done + ls -la "$dest" 2>/dev/null || echo "no crash reports found" + - name: Upload failing test artifacts if: failure() uses: actions/upload-artifact@v6 @@ -158,5 +175,6 @@ jobs: http-server/tests/phpt/**/*.out http-server/tests/phpt/**/*.diff http-server/tests/phpt/**/*.log + http-server/tests/phpt/crash-reports/* if-no-files-found: ignore retention-days: 7 diff --git a/src/http_server_class.c b/src/http_server_class.c index 20ad9d6..0ae4995 100644 --- a/src/http_server_class.c +++ b/src/http_server_class.c @@ -1888,7 +1888,8 @@ static void http_server_accept_callback( * releases the array. */ typedef struct { - zval server_transit; /* per-worker persistent shell */ + zval server_transit; /* per-worker persistent shell */ + zend_async_event_t *done_event; /* completion future from submit_internal — owned here */ } pool_worker_ctx_t; static void http_server_release_worker_shell(zval *transit); @@ -1949,6 +1950,14 @@ static void pool_worker_handler(zend_async_event_t *event, void *vctx) * by http_server_free when the parent server destructs. */ } +/* No-op: st->cb is embedded in pool_await_state and shared, so a disposed + * future's callbacks_free() must not free it (a NULL dispose would also crash). */ +static void pool_worker_cb_dispose(zend_async_event_callback_t *cb, + zend_async_event_t *event) +{ + (void)cb; (void)event; +} + static void pool_worker_done_cb(zend_async_event_t *event, zend_async_event_callback_t *cb, void *result, zend_object *exception) @@ -2785,6 +2794,7 @@ static int http_server_start_pool(http_server_object *server, pool_worker_ctx_t *ctxs = pemalloc(sizeof(*ctxs) * (size_t)workers, 1); for (int i = 0; i < workers; i++) { ZVAL_UNDEF(&ctxs[i].server_transit); + ctxs[i].done_event = NULL; ZEND_ASYNC_THREAD_TRANSFER_ZVAL_TOPLEVEL(&ctxs[i].server_transit, this_zv); if (UNEXPECTED(EG(exception))) { @@ -2819,6 +2829,7 @@ static int http_server_start_pool(http_server_object *server, st->pending = workers; st->all_done = create_server_wait_event(); st->cb.callback = pool_worker_done_cb; + st->cb.dispose = pool_worker_cb_dispose; /* Retained for HttpServer::reload() (issue #93): it rotates worker_pool and * balances pool_await_state->pending for the fresh cohort. Cleared below @@ -2842,6 +2853,7 @@ static int http_server_start_pool(http_server_object *server, } zend_async_callbacks_push(worker_evt, &st->cb); + ctxs[i].done_event = worker_evt; } server->in_pool_mode = true; @@ -2868,6 +2880,10 @@ static int http_server_start_pool(http_server_object *server, zend_async_resume_when(coroutine, st->all_done, true, zend_async_waker_callback_resolve, NULL); ZEND_ASYNC_SUSPEND(); + /* resume_when(..., true) handed all_done to the waker; waker_clean + * disposes it. NULL our pointer so the cleanup dispose below is a no-op + * — disposing again is a use-after-free (same as server->wait_event). */ + st->all_done = NULL; zend_async_waker_clean(coroutine); if (EG(exception)) { @@ -2878,6 +2894,22 @@ static int http_server_start_pool(http_server_object *server, rc = (st->pending == 0) ? SUCCESS : FAILURE; cleanup: + /* Dispose the completion futures we own, else their cross-thread triggers + * linger armed on our reactor. Live cohort, not `ctxs` — reload() swaps it. */ + { + pool_worker_ctx_t *cur = (pool_worker_ctx_t *) server->pool_worker_ctx; + for (int i = 0; cur != NULL && i < server->pool_worker_ctx_count; i++) { + zend_async_event_t *ev = cur[i].done_event; + + if (ev == NULL) { + continue; + } + + cur[i].done_event = NULL; + ZEND_ASYNC_EVENT_RELEASE(ev); + } + } + server->running = false; server->stopping = false; server->in_pool_mode = false; @@ -3797,6 +3829,7 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) for (int i = 0; i < workers; i++) { ZVAL_UNDEF(&fresh[i].server_transit); + fresh[i].done_event = NULL; ZEND_ASYNC_THREAD_TRANSFER_ZVAL_TOPLEVEL(&fresh[i].server_transit, ZEND_THIS); if (UNEXPECTED(EG(exception))) { @@ -3864,6 +3897,7 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) } zend_async_callbacks_push(worker_evt, &st->cb); + fresh[i].done_event = worker_evt; submitted++; } @@ -3877,6 +3911,13 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) pool_worker_ctx_t *old_ctxs = server->pool_worker_ctx; for (int i = 0; i < workers; i++) { + /* Dispose the retired cohort's completion futures per rotation, so they + * don't accumulate on the parent reactor across reloads. */ + if (old_ctxs[i].done_event != NULL) { + ZEND_ASYNC_EVENT_RELEASE(old_ctxs[i].done_event); + old_ctxs[i].done_event = NULL; + } + http_server_release_worker_shell(&old_ctxs[i].server_transit); } diff --git a/tests/phpt/server/core/055-pool-graceful-shutdown.phpt b/tests/phpt/server/core/055-pool-graceful-shutdown.phpt new file mode 100644 index 0000000..0ddbba6 --- /dev/null +++ b/tests/phpt/server/core/055-pool-graceful-shutdown.phpt @@ -0,0 +1,59 @@ +--TEST-- +HttpServer: graceful_shutdown() drains the worker pool and start() returns cleanly — no loop-alive assert (#93) +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- +/dev/null')) die('skip curl CLI not available'); +?> +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(5) + ->setWriteTimeout(5) + ->setWorkers(3) + ->setBootloader(static function (): void {}); + +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { + $res->setStatusCode(200)->setBody('pong'); +}); + +spawn(function () use ($port) { + $curl = static fn (): string => (string) shell_exec(sprintf( + 'curl -s --max-time 2 http://127.0.0.1:%d/ 2>/dev/null', $port)); + + $up = ''; + for ($i = 0; $i < 50 && $up !== 'pong'; $i++) { + usleep(200000); + $up = $curl(); + } + + echo "up=", $up, "\n"; + + \Async\graceful_shutdown(); +}); + +$server->start(); +echo "stopped=clean\n"; +?> +--EXPECTF-- +up=pong%A +stopped=clean%A diff --git a/tests/phpt/server/core/056-pool-shutdown-timeout-zero.phpt b/tests/phpt/server/core/056-pool-shutdown-timeout-zero.phpt new file mode 100644 index 0000000..15c5223 --- /dev/null +++ b/tests/phpt/server/core/056-pool-shutdown-timeout-zero.phpt @@ -0,0 +1,68 @@ +--TEST-- +HttpServer: setShutdownTimeout(0) — graceful_shutdown() quiesces the pool immediately, no drain wait, clean exit (#93) +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- +/dev/null')) die('skip curl CLI not available'); +?> +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(5) + ->setWriteTimeout(5) + ->setWorkers(3) + ->setShutdownTimeout(0) + ->setBootloader(static function (): void { + \Async\spawn(static function (): void { + while (true) { + \Async\delay(200); + } + }); + }); + +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { + $res->setStatusCode(200)->setBody('pong'); +}); + +spawn(function () use ($port) { + $curl = static fn (): string => (string) shell_exec(sprintf( + 'curl -s --max-time 2 http://127.0.0.1:%d/ 2>/dev/null', $port)); + + $up = ''; + for ($i = 0; $i < 50 && $up !== 'pong'; $i++) { + usleep(200000); + $up = $curl(); + } + + echo "up=", $up, "\n"; + + \Async\graceful_shutdown(); +}); + +$server->start(); +echo "stopped=clean\n"; +?> +--EXPECTF-- +up=pong%A +stopped=clean%A diff --git a/tests/phpt/server/core/057-pool-reload-then-shutdown.phpt b/tests/phpt/server/core/057-pool-reload-then-shutdown.phpt new file mode 100644 index 0000000..29bae68 --- /dev/null +++ b/tests/phpt/server/core/057-pool-reload-then-shutdown.phpt @@ -0,0 +1,64 @@ +--TEST-- +HttpServer: reload() then graceful_shutdown() — rotated + final cohorts both reaped, clean exit (#93) +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- +/dev/null')) die('skip curl CLI not available'); +?> +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(5) + ->setWriteTimeout(5) + ->setWorkers(3) + ->setBootloader(static function (): void {}); + +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { + $res->setStatusCode(200)->setBody('pong'); +}); + +spawn(function () use ($server, $port) { + $curl = static fn (): string => (string) shell_exec(sprintf( + 'curl -s --max-time 2 http://127.0.0.1:%d/ 2>/dev/null', $port)); + + $up = ''; + for ($i = 0; $i < 50 && $up !== 'pong'; $i++) { + usleep(200000); + $up = $curl(); + } + echo "up=", $up, "\n"; + + $server->reload(); + echo "reloaded\n"; + + for ($i = 0; $i < 50 && $curl() !== 'pong'; $i++) { + usleep(200000); + } + + \Async\graceful_shutdown(); +}); + +$server->start(); +echo "stopped=clean\n"; +?> +--EXPECTF-- +up=pong%A +reloaded%A +stopped=clean%A