refactor(server): 构造路径原子化 —— out-param 一次建立、单一移交点、错误码如实

include/uvhttp_server.h

@@ -77,13 +77,18 @@ typedef struct {
     uvhttp_request_handler_t handler;
 } uvhttp_route_t;
 
-/* Server builder structure (unified API) */
+/* Server builder structure (unified API)
+ *
+ * server/router/config are handles the builder exposes for the caller's
+ * convenience; the builder owns none of them - uvhttp_server_free releases
+ * all three. The struct is therefore only a view, never an owner, which is
+ * why a partially built builder is safe to drop with uvhttp_free() alone
+ * (see create_simple_server_internal). */
 typedef struct {
     uvhttp_server_t* server;
     uvhttp_router_t* router;
     uvhttp_config_t* config;
     uv_loop_t* loop;
-    int auto_cleanup;
 } uvhttp_server_builder_t;
 /* ========== Timeout Statistics Callback ========== */
 /**

src/uvhttp_server.c

@@ -898,73 +898,78 @@ static uvhttp_error_t create_simple_server_internal(
     if (!server) {
         return UVHTTP_ERROR_INVALID_PARAM;
     }
-
-    uvhttp_server_builder_t* simple =
-        uvhttp_alloc(sizeof(uvhttp_server_builder_t));
-    if (!simple) {
-        return UVHTTP_ERROR_OUT_OF_MEMORY;
-    }
-
-    memset(simple, 0, sizeof(uvhttp_server_builder_t));
+    /* One out-param invariant, established once here instead of being
+     * re-asserted in every failure branch: the caller either gets a fully
+     * listening builder or NULL. */
+    *server = NULL;
 
     // Loop must be provided by application layer
     if (!loop) {
         UVHTTP_LOG_ERROR("loop parameter is required - must be provided by "
                          "application layer");
-        uvhttp_free(simple);
-        *server = NULL;
         return UVHTTP_ERROR_INVALID_PARAM;
     }
+
+    uvhttp_server_builder_t* simple =
+        uvhttp_alloc(sizeof(uvhttp_server_builder_t));
+    if (!simple) {
+        return UVHTTP_ERROR_OUT_OF_MEMORY;
+    }
+    memset(simple, 0, sizeof(uvhttp_server_builder_t));
     simple->loop = loop;
 
-    // Create server
-    uvhttp_error_t server_result =
-        uvhttp_server_new(simple->loop, &simple->server);
-    if (server_result != UVHTTP_OK) {
+    /* Each sub-object is owned by a local until the single handover point
+     * below. Every failure between here and there frees its own object by
+     * name, so no branch ever has to "un-own" anything from the server -
+     * which is what the previous shape required (it nulled server->config /
+     * server->router on some branches and not others, and needed a comment
+     * warning against "fixing" the one branch where nulling would leak). */
+    uvhttp_server_t* srv = NULL;
+    uvhttp_error_t err = uvhttp_server_new(loop, &srv);
+    if (err != UVHTTP_OK) {
         uvhttp_free(simple);
-        *server = NULL;  // set to NULL to avoid double release
-        return server_result;
+        return err;
     }
 
-    // Create router
-    uvhttp_error_t router_result = uvhttp_router_new(&simple->router);
-    if (router_result != UVHTTP_OK) {
-        // Before calling uvhttp_server_free, set config to NULL
-        simple->server->config = NULL;
-        uvhttp_server_free(simple->server);
+    uvhttp_router_t* router = NULL;
+    err = uvhttp_router_new(&router);
+    if (err != UVHTTP_OK) {
+        uvhttp_server_free(srv);
         uvhttp_free(simple);
-        *server = NULL;  // set to NULL to avoid double release
-        return router_result;
+        return err;
     }
 
-    // Create and set default config
-    uvhttp_error_t result = uvhttp_config_new(&simple->config);
-    if (result != UVHTTP_OK) {
-        // Before calling uvhttp_server_free, set config and router to NULL
-        simple->server->config = NULL;
-        simple->server->router = NULL;
-        uvhttp_router_free(simple->router);
-        uvhttp_server_free(simple->server);
+    uvhttp_config_t* config = NULL;
+    err = uvhttp_config_new(&config);
+    if (err != UVHTTP_OK) {
+        uvhttp_router_free(router);
+        uvhttp_server_free(srv);
         uvhttp_free(simple);
-        *server = NULL;  // set to NULL to avoid double release
-        return result;
+        return err;
     }
 
-    simple->server->config = simple->config;
-    simple->server->router = simple->router;
-    simple->auto_cleanup = 1;
+    /* Ownership handover: exactly one point, reached only on the path that
+     * built everything. From here on, uvhttp_server_free releases srv, the
+     * router and the config; the builder struct holds the handles for the
+     * caller's convenience but owns none of them. */
+    simple->server = srv;
+    simple->router = router;
+    simple->config = config;
+    srv->config = config;
+    srv->router = router;
 
     // startlisten
-    if (uvhttp_server_listen(simple->server, host, port) != UVHTTP_OK) {
-        UVHTTP_LOG_ERROR("Failed to start server on %s:%d\n", host, port);
-        // By this point simple->server->config and simple->server->router have
-        // already been assigned (above), so uvhttp_server_free will release
-        // them. Do NOT null them here - that would defeat the free and leak
-        // the router (and its internal allocations).
-        uvhttp_server_free(simple->server);
+    err = uvhttp_server_listen(srv, host, port);
+    if (err != UVHTTP_OK) {
+        UVHTTP_LOG_ERROR("Failed to start server on %s:%d\n", host ? host : "",
+                         port);
+        /* Propagate the real reason instead of collapsing every listen
+         * failure into SERVER_LISTEN: a NULL host is a caller mistake
+         * (INVALID_PARAM), an occupied port is a runtime one
+         * (SERVER_LISTEN), and the two deserve different remediation. */
+        uvhttp_server_free(srv);
         uvhttp_free(simple);
-        *server = NULL;  // set to NULL to avoid double release
-        return UVHTTP_ERROR_SERVER_LISTEN;
+        return err;
     }
 
     *server = simple;

test/unit/test_server_atomic_construct.cpp

@@ -415,3 +415,94 @@ TEST(UvhttpServerOwnership, NullContextWithoutContextIsNoOp) {
 
     uvhttp_server_free(server);
 }
+
+/* ---------- builder 构造路径（create_simple_server_internal） ---------- */
+
+/*
+ * v2.10 d5：builder 构造路径原来在 5 个分支里各自 `*server = NULL`，并在两个
+ * 分支里手工置 `server->config/router = NULL`（其中三处其实是无操作——那两项
+ * 的所有权要到后面才移交），还靠一条"Do NOT null them here"的注释防止后人
+ * "修好"唯一不能置空的分支。
+ *
+ * 现在：out-param 在函数头部一次性建立；子对象由局部变量持有到唯一的移交点；
+ * 每个失败分支只释放自己命名的对象；移交之后 server_free 一并释放三者。
+ * 下面锁住这两条不变式。
+ */
+
+/* 所有失败路径上 out-param 必须是 NULL（此前未断言过，只是各分支自觉置）。
+ * 用哨兵值而非 nullptr 初始化：否则"函数压根没写 out-param"也照样绿。 */
+TEST(UvhttpServerAtomicConstruct, CreateBuilderFailureLeavesOutParamNull) {
+    uvhttp_server_builder_t* builder =
+        reinterpret_cast<uvhttp_server_builder_t*>(0x1);
+
+    EXPECT_EQ(uvhttp_server_create(nullptr, "127.0.0.1", 8080, &builder),
+              UVHTTP_ERROR_INVALID_PARAM);
+    EXPECT_EQ(builder, nullptr) << "loop 缺失时 out-param 应被写成 NULL";
+
+    uv_loop_t* loop = uv_default_loop();
+    ASSERT_NE(loop, nullptr);
+
+    builder = reinterpret_cast<uvhttp_server_builder_t*>(0x1);
+    /* host 为 NULL：uvhttp_server_listen 自己拒绝，构造路径据此回滚 */
+    EXPECT_EQ(uvhttp_server_create(loop, nullptr, 8080, &builder),
+              UVHTTP_ERROR_INVALID_PARAM);
+    EXPECT_EQ(builder, nullptr) << "host 缺失时 out-param 应被写成 NULL";
+}
+
+/* listen 失败（端口被占）：server 已经接管 router 与 config，清理必须只走
+ * server_free —— 这一条正是旧五步法 double free 的同款路径。端口占用用
+ * uv_listen 占位实现，与运行身份无关（特权端口在 root 下会绑定成功）。
+ * 错误码必须是 SERVER_LISTEN（运行时占用），不是 INVALID_PARAM。 */
+TEST(UvhttpServerAtomicConstruct, CreateBuilderListenFailureLeavesNothing) {
+    uv_loop_t* loop = uv_default_loop();
+    ASSERT_NE(loop, nullptr);
+
+    uv_tcp_t blocker;
+    ASSERT_EQ(uv_tcp_init(loop, &blocker), 0);
+    struct sockaddr_in bind_addr;
+    ASSERT_EQ(uv_ip4_addr("127.0.0.1", 0, &bind_addr), 0);
+    ASSERT_EQ(uv_tcp_bind(&blocker, (const struct sockaddr*)&bind_addr, 0), 0);
+    ASSERT_EQ(uv_listen((uv_stream_t*)&blocker, 1, [](uv_stream_t*, int) {}),
+              0);
+    struct sockaddr_in bound_addr;
+    int addr_len = sizeof(bound_addr);
+    ASSERT_EQ(
+        uv_tcp_getsockname(&blocker, (struct sockaddr*)&bound_addr, &addr_len),
+        0);
+    const int occupied_port = ntohs(bound_addr.sin_port);
+    ASSERT_GT(occupied_port, 0);
+
+    uvhttp_server_builder_t* builder = nullptr;
+    const uvhttp_error_t err =
+        uvhttp_server_create(loop, "127.0.0.1", occupied_port, &builder);
+
+    EXPECT_EQ(err, UVHTTP_ERROR_SERVER_LISTEN);
+    EXPECT_EQ(builder, nullptr) << "listen 失败时不得交出半成品 builder（其 "
+                                   "router/config 已归 server 所有）";
+
+    uv_close((uv_handle_t*)&blocker, nullptr);
+    uv_run(loop, UV_RUN_NOWAIT);
+}
+
+/* 成功路径的唯一移交点：builder 是视图不是所有者 —— 三个句柄必须指向
+ * server 持有的同一批对象（移交与"server 侧接管"是同一个动作）。 */
+TEST(UvhttpServerAtomicConstruct, BuilderIsAViewNotAnOwner) {
+    uv_loop_t* loop = uv_default_loop();
+    ASSERT_NE(loop, nullptr);
+
+    uvhttp_server_builder_t* builder = nullptr;
+    ASSERT_EQ(uvhttp_server_create(loop, "127.0.0.1", 0, &builder), UVHTTP_OK);
+    ASSERT_NE(builder, nullptr);
+
+    EXPECT_EQ(builder->server->config, builder->config)
+        << "config 必须在同一个移交点交给 server";
+    EXPECT_EQ(builder->server->router, builder->router)
+        << "router 必须在同一个移交点交给 server";
+    EXPECT_NE(builder->server->config, nullptr);
+    EXPECT_NE(builder->server->router, nullptr);
+    EXPECT_EQ(builder->loop, loop);
+
+    /* simple_free 只释放 server（连带 router/config）与 builder 自身；
+     * 若 builder 也拥有这些对象，这里就是 double free（ASan 会报）。 */
+    uvhttp_server_simple_free(builder);
+}