From b9839d59513a5fe64b7598f3923a438ff8181755 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Mon, 1 Jun 2026 11:15:12 +0000 Subject: [PATCH] http: drain in-flight per-request coroutines on shutdown (#74) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server shutdown disposed server_scope while per-request handler coroutines were still suspended, which aborts in debug builds (scope.c "Scope should be empty before disposal") and otherwise leaves the worker hanging on the parked coroutines. start() now drains server_scope on its lifecycle coroutine right after stop() wakes it (never inside stop(), which may be called from a handler living in server_scope), using the Scope's own orderly API: awaitCompletion() within the shutdown-timeout grace window, then cancel() + awaitAfterCancellation() for whatever is still parked. A guarded fallback runs in http_server_free for the freed-mid-flight path. server_scope is created with DISPOSE_SAFELY cleared so cancel() hard-terminates the handlers instead of zombifying them — a safe scope would zombify them and awaitAfterCancellation would wait forever. The grace window reuses the existing setShutdownTimeout knob (default 5s; 0 = cancel immediately). Adds tests/phpt/server/core/045-shutdown-drains-inflight.phpt. --- src/http_server_class.c | 127 ++++++++++++++++++ .../core/045-shutdown-drains-inflight.phpt | 66 +++++++++ 2 files changed, 193 insertions(+) create mode 100644 tests/phpt/server/core/045-shutdown-drains-inflight.phpt diff --git a/src/http_server_class.c b/src/http_server_class.c index adc7d58..ad113cc 100644 --- a/src/http_server_class.c +++ b/src/http_server_class.c @@ -387,6 +387,10 @@ struct http_server_object { bool running; bool stopping; bool listeners_paused; + /* Set once start()'s post-wakeup drain has emptied server_scope, so the + * http_server_free fallback drain is skipped on the normal stop() path + * (issue #74). */ + bool scope_drained; /* Set in transfer_obj LOAD when this object was constructed by the * built-in worker pool (issue #11) — start() skips re-spawning the * pool and runs the standalone event loop. */ @@ -1486,6 +1490,112 @@ static zend_async_event_t *create_server_wait_event(void) return event; } +/* Build an Async\Timeout awaitable for `ms` — the cancellation token + * Scope::awaitCompletion() expects. Returns false (out left UNDEF) on failure. */ +static bool http_server_make_timeout(const zend_ulong ms, zval *out) +{ + zval fname, arg; + ZVAL_STRINGL(&fname, "Async\\timeout", sizeof("Async\\timeout") - 1); + ZVAL_LONG(&arg, (zend_long) ms); + ZVAL_UNDEF(out); + + const bool ok = call_user_function(NULL, NULL, &fname, out, 1, &arg) == SUCCESS + && EG(exception) == NULL + && Z_TYPE_P(out) == IS_OBJECT; + + zval_ptr_dtor(&fname); + + if (false == ok) { + zval_ptr_dtor(out); + ZVAL_UNDEF(out); + + if (EG(exception)) { + zend_clear_exception(); + } + } + + return ok; +} + +/* Async\Scope::isFinished() on the live scope object. */ +static bool http_server_scope_is_finished(zend_object *scope_object) +{ + zval retval; + ZVAL_UNDEF(&retval); + zend_call_method_with_0_params(scope_object, scope_object->ce, NULL, "isFinished", &retval); + + const bool finished = (Z_TYPE(retval) == IS_TRUE); + zval_ptr_dtor(&retval); + return finished; +} + +/* Call a no-arg, void Scope method, swallowing any exception — a cancellation + * thrown during shutdown is expected, not an error. */ +static void http_server_scope_call(zend_object *scope_object, const char *method) +{ + zval retval; + ZVAL_UNDEF(&retval); + zend_call_method_with_0_params(scope_object, scope_object->ce, NULL, method, &retval); + zval_ptr_dtor(&retval); + + if (EG(exception)) { + zend_clear_exception(); + } +} + +/* Graceful-shutdown drain (issue #74): empty server_scope before it is + * disposed. Runs on the start() coroutine after stop() wakes it — not inside + * stop(), which may be called from a handler that lives in server_scope. Uses + * the Scope's own API (awaitCompletion within the grace window, then cancel + + * awaitAfterCancellation); these own the scope lifetime, so nothing dangles. */ +static void http_server_drain_scope(http_server_object *server) +{ + if (server->scope_object == NULL || server->server_scope == NULL || server->scope_drained) { + return; + } + + /* Need a real coroutine to suspend on. In scheduler/teardown context we + * cannot await — leave scope_drained unset so a later call may retry. */ + if (ZEND_ASYNC_CURRENT_COROUTINE == NULL || ZEND_ASYNC_IS_SCHEDULER_CONTEXT + || false == ZEND_ASYNC_ON) { + return; + } + + zend_object *scope_object = server->scope_object; + GC_ADDREF(scope_object); /* hold the Scope object across the method calls */ + + /* Phase 1: let handlers finish on their own, bounded by the grace window. */ + const zend_ulong grace_ms = (zend_ulong) server->shutdown_timeout_s * 1000u; + + if (grace_ms > 0 && false == http_server_scope_is_finished(scope_object)) { + zval timeout; + + if (http_server_make_timeout(grace_ms, &timeout)) { + zval retval; + ZVAL_UNDEF(&retval); + zend_call_method_with_1_params(scope_object, scope_object->ce, NULL, + "awaitCompletion", &retval, &timeout); + zval_ptr_dtor(&retval); + zval_ptr_dtor(&timeout); + + if (EG(exception)) { + zend_clear_exception(); /* grace expired, or completed during the wait */ + } + } + } + + /* Phase 2: cancel whatever is still parked, then await the cancellation. + * server_scope is non-dispose-safely (see start()), so cancel() hard- + * terminates the handlers rather than zombifying them. */ + if (false == http_server_scope_is_finished(scope_object)) { + http_server_scope_call(scope_object, "cancel"); + http_server_scope_call(scope_object, "awaitAfterCancellation"); + } + + server->scope_drained = true; + OBJ_RELEASE(scope_object); +} + /* Accept callback — fired once per accepted connection by the reactor. * `result` points to the accepted client socket fd (zend_socket_t) the * reactor extracted from libuv; `exception` is set on accept failures. @@ -2304,6 +2414,13 @@ ZEND_METHOD(TrueAsync_HttpServer, start) "Failed to create server scope", 0); RETURN_FALSE; } + + /* Hard-terminate handler coroutines at shutdown instead of zombifying them: + * clear DISPOSE_SAFELY (inherited from the main scope), else cancel() leaves + * zombies and the shutdown drain's awaitAfterCancellation waits forever + * (issue #74). Child request scopes inherit this non-safe mode. */ + ZEND_ASYNC_SCOPE_CLR_DISPOSE_SAFELY(server->server_scope); + /* Keep our own pointer to the scope's zend_object. scope_destroy (the * dtor_obj handler) runs during request shutdown's dtor phase BEFORE * our http_server_free (the free_obj handler) runs — and it nulls @@ -2748,6 +2865,11 @@ ZEND_METHOD(TrueAsync_HttpServer, start) zend_bailout(); } + /* Drain in-flight per-request handler coroutines now, while we are still + * on the start() coroutine and can suspend, so server_scope is empty when + * the object is freed (issue #74). On bailout we already longjmp'd above. */ + http_server_drain_scope(server); + RETURN_TRUE; } /* }}} */ @@ -3347,6 +3469,11 @@ static void http_server_free(zend_object *obj) } } + /* Fallback drain (issue #74): if start()'s drain never ran (freed mid-flight, + * or stop() never called) and we can still suspend, empty the scope here. + * No-op in teardown/scheduler context — the OBJ_RELEASE below cancels the rest. */ + http_server_drain_scope(server); + /* Release the scope_object we took at scope creation in start(). * * scope_destroy (the object dtor) will clear scope->scope_object = NULL diff --git a/tests/phpt/server/core/045-shutdown-drains-inflight.phpt b/tests/phpt/server/core/045-shutdown-drains-inflight.phpt new file mode 100644 index 0000000..0398f56 --- /dev/null +++ b/tests/phpt/server/core/045-shutdown-drains-inflight.phpt @@ -0,0 +1,66 @@ +--TEST-- +HttpServer: graceful shutdown drains in-flight per-request coroutines (issue #74) +--EXTENSIONS-- +true_async_server +true_async +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(10) + ->setWriteTimeout(10) + ->setShutdownTimeout(0); // no grace: force-cancel whatever is parked + +$server = new HttpServer($config); + +$server->addHttpHandler(function ($request, $response) use ($server) { + $uri = $request->getUri(); + if (str_starts_with($uri, '/park')) { + Async\request_context()->set('rid', ltrim($uri, '/')); + Async\delay(60000); // stays parked across shutdown + $response->setStatusCode(200)->setBody("late\n")->end(); + return; + } + $response->setStatusCode(200)->setBody("stopping\n")->end(); + $server->stop(); +}); + +$client = spawn(function () use ($port) { + usleep(40000); + // Fire the parked request first (do not wait for its response). + $park = @stream_socket_client("tcp://127.0.0.1:$port", $e, $s, 2); + if ($park) { + fwrite($park, "GET /park HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + } + usleep(40000); + // Now stop the server while /park is still suspended. + $stop = @stream_socket_client("tcp://127.0.0.1:$port", $e, $s, 2); + if ($stop) { + fwrite($stop, "GET /stop HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + $buf = ''; + while (!feof($stop)) { $buf .= fread($stop, 8192); } + fclose($stop); + echo str_contains($buf, "stopping") ? "stop responded\n" : "stop missing\n"; + } + if ($park) { @fclose($park); } +}); + +$server->start(); +await($client); +echo "server stopped cleanly\n"; +?> +--EXPECT-- +stop responded +server stopped cleanly