test: cover the web layer, which the sanitizer build only appeared to…

.github/workflows/code-quality.yml

@@ -107,7 +107,12 @@ jobs:
       run: |
         ctest --output-on-failure --verbose
       env:
-        ASAN_OPTIONS: detect_leaks=1:halt_on_error=1
+        # exitcode=86 distinguishes a leak from a failing assertion: LSan's default
+        # is 1, the same code a test returns when it fails, so the two are otherwise
+        # indistinguishable in the log. halt_on_error=1 matters just as much as
+        # detect_leaks=1 -- with detect_leaks alone the leak is reported but the
+        # process still exits 0, which CI reads as a pass.
+        ASAN_OPTIONS: detect_leaks=1:halt_on_error=1:exitcode=86
         LSAN_OPTIONS: suppressions=../lsan.supp
 
   # Undefined Behavior Sanitizer
@@ -359,15 +364,15 @@ jobs:
       run: |
         ctest -L fuzz --output-on-failure -E FuzzSmoke
       env:
-        ASAN_OPTIONS: detect_leaks=1:halt_on_error=1
+        ASAN_OPTIONS: detect_leaks=1:halt_on_error=1:exitcode=86
         LSAN_OPTIONS: suppressions=../lsan.supp
 
     - name: Fuzz smoke test
       working-directory: build
       run: |
         ctest -L fuzz --output-on-failure -R FuzzSmoke
       env:
-        ASAN_OPTIONS: detect_leaks=1:halt_on_error=1
+        ASAN_OPTIONS: detect_leaks=1:halt_on_error=1:exitcode=86
         LSAN_OPTIONS: suppressions=../lsan.supp
 
     # A crash leaves a reproducer behind; upload it so the failure is actionable

CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 ### Added
+- `tests/test_web_layer.cpp` covers the Drogon adapter and the embedded pages end to
+  end: it starts the real server, drives it over raw-socket HTTP and shuts it down.
+  Until now every test target was core-only, so `drogon_adapter.cpp` and
+  `web_resources.cpp` were compiled under ASan but executed by nothing that ctest
+  ran -- the sanitizer build only looked like it covered the web layer. The server is
+  started once per suite because `drogon::app()` is a bind-once singleton: a
+  per-test SetUp/TearDown segfaults in `ListenerManager::addListener` on the second
+  test. Port 0 plus `getListeners()` keeps parallel runs from colliding.
+- The sanitizer steps set `ASAN_OPTIONS=...:exitcode=86`, so a leak is distinguishable
+  from a failed assertion in the log. LSan exits 1 by default, which is the same code
+  a test returns when an assertion fails.
 - libFuzzer targets for the parsers, behind `-DREMOTE_PROFILER_BUILD_FUZZERS=ON`
   (clang only, off by default): `fuzz_renderer_output`, `fuzz_result_parsing`,
   `fuzz_http_params`, with a committed seed corpus each. `ctest -L fuzz` replays the

CMakeLists.txt

@@ -308,6 +308,21 @@ if(REMOTE_PROFILER_BUILD_TESTS)
     )
     add_test(NAME ThreadStacksTest COMMAND test_thread_stacks)
 
+    # End-to-end web layer (Drogon adapter + embedded resources). Only built when
+    # the web layer is enabled, since it is the only test that links it.
+    if(REMOTE_PROFILER_ENABLE_WEB)
+        add_executable(test_web_layer tests/test_web_layer.cpp)
+        target_link_libraries(test_web_layer
+            PRIVATE
+            profiler_web
+            Drogon::Drogon
+            GTest::gtest
+            GTest::gtest_main
+            pthread
+        )
+        add_test(NAME WebLayerTest COMMAND test_web_layer)
+    endif()
+
     message(STATUS "Tests will be built")
 else()
     message(STATUS "Tests disabled")

CONTRIBUTING.md

@@ -314,6 +314,17 @@ build/fuzz/fuzz_renderer_output -max_total_time=300 tests/fuzz/corpus/fuzz_rende
 - **Sanitizer**：`ctest` 默认构建不会报内存泄漏与未定义行为。曾经有一个
   `stopHeapProfiler()` 的 LeakSanitizer 报告只被 CI 发现，因为本地从没跑过
   `-fsanitize=address`。
+
+  ⚠️ 想靠**退出码**判断有没有泄漏，两个参数都要给，只给第一个不够：
+
+  | 设置 | 泄漏报告 | 退出码 |
+  |------|----------|--------|
+  | `detect_leaks=1`（`halt_on_error` 默认 0） | ✅ 打印 | **0** ← 泄漏了但 CI 视为通过 |
+  | `detect_leaks=1:halt_on_error=1` | ✅ 打印 | 非零 ✅ |
+  | 再加 `exitcode=86` | ✅ 打印 | **86** ← 与断言失败（1）可区分 |
+
+  另外两条限制：进程被 `_exit()`/信号杀死时 LSan 没机会运行；被抑制规则覆盖的泄漏
+  （如 `lsan.supp` 里的 tcmalloc 初始化）不会让退出码非零。
 - **Web UI**：`curl` **无法执行页面 JavaScript**，因此看不到"按钮点击报
   ReferenceError""按钮卡在 disabled""下载其实没开始"这类问题。前端改动必须用
   真实浏览器验证后才算完成。

scripts/check-sanitizers.sh

@@ -58,8 +58,11 @@ run_asan() {
         -DCMAKE_MODULE_LINKER_FLAGS="-fsanitize=address" \
         "${extra_configure_args[@]}"
     cmake --build "$dir" -j"$JOBS"
-    # detect_leaks + halt_on_error: a leak must fail the run, as it does in CI.
-    ( cd "$dir" && ASAN_OPTIONS=detect_leaks=1:halt_on_error=1 \
+    # detect_leaks reports a leak; halt_on_error is what makes the process fail
+    # because of it (with detect_leaks alone the report is printed and the exit code
+    # stays 0). exitcode=86 then separates a leak from a failing assertion, since
+    # LSan's default 1 is the same code a test uses when an assertion fails.
+    ( cd "$dir" && ASAN_OPTIONS=detect_leaks=1:halt_on_error=1:exitcode=86 \
         LSAN_OPTIONS="suppressions=$PROJECT_ROOT/lsan.supp" \
         ctest --output-on-failure "${ctest_parallel[@]}" )
 }

tests/test_full_flow.cpp

@@ -16,14 +16,20 @@ namespace {
 /// profile must therefore generate real CPU work for the whole window instead of
 /// sleeping through it -- otherwise they fail whenever the machine happens to be
 /// idle, which is exactly how this test failed under parallel ctest.
-void burnCPUFor(int ms) {
+uint64_t burnCPUFor(int ms) {
     const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(ms);
-    volatile uint64_t sink = 0;
+    uint64_t sink = 0;
     while (std::chrono::steady_clock::now() < deadline) {
         for (int i = 0; i < 20000; ++i) {
             sink += static_cast<uint64_t>(i) * 2654435761u;
         }
     }
+    // Returning it rather than marking it unused: callers assert it is non-zero, so
+    // the accumulation is observable and the compiler cannot elide it. A `volatile`
+    // local would silence the optimizer but still trip -Wunused-but-set-variable
+    // (GCC sees through volatile for that warning, and the warnings-check job builds
+    // with -Wall -Wextra -Werror).
+    return sink;
 }
 
 } // namespace
@@ -38,7 +44,7 @@ TEST(FullFlowTest, GperftoolsGeneratesValidProfile) {
 
     // 运行足够的工作负载
     std::cout << "Running workload...\n";
-    volatile long long result = 0;
+    long long result = 0;
     for (int i = 0; i < 10000; ++i) {
         result += i * i;
         // 一些函数调用
@@ -47,7 +53,11 @@ TEST(FullFlowTest, GperftoolsGeneratesValidProfile) {
     }
 
     // Keep sampling real work rather than idling: see burnCPUFor().
-    burnCPUFor(500);
+    const uint64_t burned = burnCPUFor(500);
+    // Used, not merely set: without an observation the loop can be elided, and both
+    // this and `result` trip -Wunused-but-set-variable under -Werror.
+    EXPECT_GT(burned, 0u);
+    EXPECT_GT(result, 0);
 
     // 停止 profiler
     ProfilerStop();
@@ -189,7 +199,7 @@ TEST(FullFlowTest, GetRawCPUProfile) {
     // from executing threads, so a 1s window with nothing to sample returns an
     // empty profile by design. The workload used to be absent here, which made
     // the non-empty assertion below depend on machine load.
-    std::thread burner{[]() { burnCPUFor(1200); }};
+    std::thread burner{[]() { EXPECT_GT(burnCPUFor(1200), 0u); }};
 
     // 获取原始 profile 数据（采样 1 秒）
     std::string profile_data = profiler.getRawCPUProfile(1);

tests/test_logger.cpp

@@ -166,10 +166,14 @@ TEST_F(LoggerTest, ThreadSafety) {
 
     std::vector<std::thread> threads;
     for (int t = 0; t < num_threads; ++t) {
-        threads.emplace_back([this, t, messages_per_thread]() {
-            for (int i = 0; i < messages_per_thread; ++i) {
-                mock_sink_->log(LogLevel::Info, "thread_test.cpp", t * messages_per_thread + i, "threadFunc",
-                                ("Thread " + std::to_string(t) + " message " + std::to_string(i)).c_str());
+        // The capture is const-foldable, so it is not captured; and the loop
+        // variable is "msg" rather than "i" because an "i" parameter would shadow
+        // the captured messages_per_thread, leaving it unused inside the lambda
+        // (clang rejects that under -Wunused-lambda-capture with -Werror).
+        threads.emplace_back([this, t]() {
+            for (int msg = 0; msg < messages_per_thread; ++msg) {
+                mock_sink_->log(LogLevel::Info, "thread_test.cpp", t * messages_per_thread + msg, "threadFunc",
+                                ("Thread " + std::to_string(t) + " message " + std::to_string(msg)).c_str());
             }
         });
     }

tests/test_web_layer.cpp

@@ -0,0 +1,212 @@
+/// @file test_web_layer.cpp
+/// @brief End-to-end test of the Drogon adapter and the embedded web resources
+///
+/// Why this exists: every other test target is core-only (no Drogon), so
+/// `drogon_adapter.cpp` and `web_resources.cpp` were compiled under ASan but never
+/// executed by anything except the example binary, which ctest does not run. The
+/// sanitizer build therefore looked like it covered the web layer while nothing
+/// actually ran. This test starts the real server, drives it over HTTP and shuts it
+/// down, so route registration, request delivery (including the async path through
+/// AsyncExecutor and queueInLoop), the embedded pages and the shutdown sequence all
+/// execute under the running sanitizer configuration.
+///
+/// Requests are issued with a raw socket rather than libcurl, to avoid adding a
+/// dependency purely for tests. A detail that makes the assertions meaningful: the
+/// response writing happens on a *different* thread, so a response that arrives at
+/// all proves the cross-thread hand-off worked, not merely that a handler ran.
+
+#include "../include/profiler/drogon_adapter.h"
+#include "../include/profiler_manager.h"
+#include <arpa/inet.h>
+#include <chrono>
+#include <cstring>
+#include <drogon/drogon.h>
+#include <future>
+#include <gtest/gtest.h>
+#include <netinet/in.h>
+#include <stdexcept>
+#include <string>
+#include <sys/socket.h>
+#include <thread>
+#include <unistd.h>
+#include <vector>
+
+namespace {
+
+/// An HTTP response reduced to what the assertions need.
+struct HttpResponse {
+    int status = 0;
+    std::string body;
+    bool received = false;
+};
+
+/// Send one request over a fresh connection and read the whole reply.
+///
+/// Deliberately minimal: HTTP/1.1, `Connection: close`, so the reply ends when the
+/// peer closes and no content-length parsing is needed.
+HttpResponse request(uint16_t port, const std::string& method, const std::string& target,
+                     const std::string& body = {}) {
+    HttpResponse result;
+
+    const int fd = ::socket(AF_INET, SOCK_STREAM, 0);
+    if (fd < 0)
+        return result;
+    sockaddr_in addr{};
+    addr.sin_family = AF_INET;
+    addr.sin_port = htons(port);
+    inet_pton(AF_INET, "127.0.0.1", &addr.sin_addr);
+    if (::connect(fd, reinterpret_cast<sockaddr*>(&addr), sizeof(addr)) != 0) {
+        ::close(fd);
+        return result;
+    }
+
+    std::string req = method + " " + target + " HTTP/1.1\r\nHost: 127.0.0.1\r\nConnection: close\r\n";
+    if (!body.empty())
+        req += "Content-Length: " + std::to_string(body.size()) + "\r\n";
+    req += "\r\n" + body;
+    if (::write(fd, req.data(), req.size()) < 0) {
+        ::close(fd);
+        return result;
+    }
+
+    std::string raw;
+    char buf[4096];
+    ssize_t n = 0;
+    while ((n = ::read(fd, buf, sizeof(buf))) > 0)
+        raw.append(buf, static_cast<size_t>(n));
+    ::close(fd);
+
+    const auto header_end = raw.find("\r\n\r\n");
+    if (header_end == std::string::npos)
+        return result;
+
+    result.received = true;
+    const auto status_line = raw.substr(0, raw.find("\r\n"));
+    const auto first_space = status_line.find(' ');
+    if (first_space != std::string::npos)
+        result.status = std::atoi(status_line.c_str() + first_space + 1);
+    result.body = raw.substr(header_end + 4);
+    return result;
+}
+
+/// A Drogon server with the profiler routes, on an ephemeral port.
+///
+/// The server is started once per *suite*, not once per test. `drogon::app()` is a
+/// process-global singleton whose lifecycle is bind-once: calling addListener() a
+/// second time, after a previous run() has finished, segfaults inside
+/// ListenerManager. Structuring this as per-test SetUp/TearDown looked natural and
+/// crashed on the second test.
+///
+/// Port 0 lets the OS pick a free port and getListeners() reports which one, so
+/// parallel ctest runs cannot collide.
+class WebLayerTest : public ::testing::Test {
+protected:
+    static void SetUpTestSuite() {
+        profiler_ = new profiler::ProfilerManager();
+        profiler::registerDrogonHandlers(*profiler_);
+
+        drogon::app().addListener("127.0.0.1", 0);
+        server_ = std::async(std::launch::async, []() { drogon::app().run(); });
+
+        // Wait for the listener to be bound rather than sleeping a fixed amount.
+        const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10);
+        while (std::chrono::steady_clock::now() < deadline) {
+            const auto listeners = drogon::app().getListeners();
+            if (!listeners.empty()) {
+                port_ = listeners.front().toPort();
+                break;
+            }
+            std::this_thread::sleep_for(std::chrono::milliseconds(20));
+        }
+        if (port_ == 0)
+            throw std::runtime_error("server never bound a port");
+    }
+
+    static void TearDownTestSuite() {
+        if (server_.valid()) {
+            drogon::app().getLoop()->queueInLoop([]() { drogon::app().quit(); });
+            server_.wait();
+        }
+        delete profiler_;
+        profiler_ = nullptr;
+    }
+
+    static uint16_t port_;
+
+private:
+    static profiler::ProfilerManager* profiler_;
+    static std::future<void> server_;
+};
+
+uint16_t WebLayerTest::port_ = 0;
+profiler::ProfilerManager* WebLayerTest::profiler_ = nullptr;
+std::future<void> WebLayerTest::server_;
+
+} // namespace
+
+TEST_F(WebLayerTest, ServesTheEmbeddedControlPanel) {
+    // Exercises WebResources::getIndexPage(): the page is embedded as a C++ string
+    // literal, so nothing else checks that it is well-formed or that the route
+    // wiring returns it.
+    const auto resp = request(port_, "GET", "/");
+
+    ASSERT_TRUE(resp.received);
+    EXPECT_EQ(resp.status, 200);
+    EXPECT_NE(resp.body.find("<!DOCTYPE html>"), std::string::npos);
+    // The panel must reference the current API surface, not a removed path.
+    EXPECT_NE(resp.body.find("/api/pprof/"), std::string::npos);
+}
+
+TEST_F(WebLayerTest, ServesStatusAsJson) {
+    const auto resp = request(port_, "GET", "/api/status");
+
+    ASSERT_TRUE(resp.received);
+    EXPECT_EQ(resp.status, 200);
+    EXPECT_NE(resp.body.find("\"cpu\""), std::string::npos);
+    EXPECT_NE(resp.body.find("\"heap\""), std::string::npos);
+}
+
+TEST_F(WebLayerTest, ServesThreadStacksThroughTheSignalPath) {
+    const auto resp = request(port_, "GET", "/api/thread/stacks");
+
+    ASSERT_TRUE(resp.received);
+    EXPECT_EQ(resp.status, 200);
+    // The capture machinery ran, which means the signal handler and the shared
+    // stack buffer were exercised on a request path.
+    EXPECT_NE(resp.body.find("Thread Call Stacks"), std::string::npos);
+}
+
+TEST_F(WebLayerTest, ServesAAnalysisRequestThroughTheAsyncExecutor) {
+    // The analysis endpoints are the ones that go through AsyncExecutor and come
+    // back via queueInLoop, i.e. the response is written from another thread. That
+    // makes this the case most likely to leak or race, and growth is used because
+    // it needs neither TCMALLOC_SAMPLE_PARAMETER nor a sampling window.
+    const auto resp = request(port_, "GET", "/api/pprof/growth?renderer=flamegraph");
+
+    ASSERT_TRUE(resp.received);
+    // Either a rendered graph or a reported failure, but the reply itself proves the
+    // cross-thread hand-off completed.
+    EXPECT_TRUE(resp.status == 200 || resp.status == 500) << "status: " << resp.status;
+    // Braces are required here, not stylistic: the EXPECT_TRUE above expands to an
+    // if/else, so an unbraced if would dangle and GCC rejects it under
+    // -Wall -Wextra -Werror (the warnings-check job).
+    if (resp.status == 200) {
+        EXPECT_NE(resp.body.find("<svg"), std::string::npos);
+    }
+}
+
+TEST_F(WebLayerTest, DeliversPprofSymbolRequests) {
+    // The only POST route, and the only one whose body is parsed as addresses.
+    const auto resp = request(port_, "POST", "/pprof/symbol", "0x401d8a+0x401d8b");
+
+    ASSERT_TRUE(resp.received);
+    EXPECT_EQ(resp.status, 200);
+    EXPECT_NE(resp.body.find("0x401d8a"), std::string::npos);
+}
+
+TEST_F(WebLayerTest, ReportsUnknownRoutesAsNotFound) {
+    const auto resp = request(port_, "GET", "/api/does-not-exist");
+
+    ASSERT_TRUE(resp.received);
+    EXPECT_EQ(resp.status, 404);
+}