fix(pool): dispose per-worker completion futures on shutdown/reload — no leaked cross-thread triggers (#93)

.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

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);
     }
 

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--
+<?php
+if (PHP_OS_FAMILY === 'Windows') die('skip libuv on Windows lacks SO_REUSEPORT');
+if (!exec('curl --version 2>/dev/null')) die('skip curl CLI not available');
+?>
+--FILE--
+<?php
+/* A pool-parent HttpServer awaits worker completion. On Async\graceful_shutdown()
+ * the workers drain and exit; the parent's bounded shutdown drain reaps their
+ * per-worker completion futures (whose cross-thread triggers would otherwise
+ * linger armed on the parent reactor). start() then returns and the process
+ * exits cleanly — on a debug build a leaked trigger would trip the scheduler's
+ * loop-alive assert (scheduler.c) and abort before "stopped=clean" prints. */
+
+use TrueAsync\HttpServer;
+use TrueAsync\HttpServerConfig;
+use function Async\spawn;
+
+require __DIR__ . '/../_free_port.inc';
+$port = tas_free_port();
+
+$config = (new HttpServerConfig())
+    ->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

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--
+<?php
+if (PHP_OS_FAMILY === 'Windows') die('skip libuv on Windows lacks SO_REUSEPORT');
+if (!exec('curl --version 2>/dev/null')) die('skip curl CLI not available');
+?>
+--FILE--
+<?php
+/* shutdown_timeout == 0 means "do not wait for workers to drain". The parent
+ * skips the bounded drain loop and quiesces the per-worker completion futures
+ * straight away: resolved ones are disposed, any still-armed one has its
+ * trigger stopped (unref, handle left open so a late cross-thread signal is a
+ * safe no-op). Either way the loop goes quiescent and start() returns without
+ * tripping the scheduler loop-alive assert. Workers also run a persistent
+ * background coroutine to prove graceful_shutdown cancels it rather than
+ * wedging teardown. */
+
+use TrueAsync\HttpServer;
+use TrueAsync\HttpServerConfig;
+use function Async\spawn;
+
+require __DIR__ . '/../_free_port.inc';
+$port = tas_free_port();
+
+$config = (new HttpServerConfig())
+    ->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

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--
+<?php
+if (PHP_OS_FAMILY === 'Windows') die('skip libuv on Windows lacks SO_REUSEPORT');
+if (!exec('curl --version 2>/dev/null')) die('skip curl CLI not available');
+?>
+--FILE--
+<?php
+/* Rotate the pool once (reload disposes the retired cohort's completion
+ * futures), then graceful_shutdown() (the final cohort's futures are disposed
+ * on the way out). Exercises both disposal sites together — a leak or a
+ * cross-thread race on either would trip the loop-alive assert / abort. */
+
+use TrueAsync\HttpServer;
+use TrueAsync\HttpServerConfig;
+use function Async\spawn;
+
+require __DIR__ . '/../_free_port.inc';
+$port = tas_free_port();
+
+$config = (new HttpServerConfig())
+    ->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