Merge remote-tracking branch 'origin/main'

Author: EdmondDantes

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

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--
+<?php
+// A handler suspended in a per-request child scope must not be left in flight
+// when the server scope is disposed at shutdown. With setShutdownTimeout(0)
+// the still-parked /park handler is force-cancelled; a /work handler that
+// finishes inside the grace window completes normally. Without the drain the
+// process would hang on the parked coroutine (or abort in a debug build).
+use TrueAsync\HttpServer;
+use TrueAsync\HttpServerConfig;
+use function Async\spawn;
+use function Async\await;
+
+$port = 20180 + getmypid() % 40;
+
+$config = (new HttpServerConfig())
+    ->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