Skip to content

Commit 1bb47bd

Browse files
etrclaude
andcommitted
ci: fix lint IWYU + latent CCN gate and cap concurrency stress under valgrind
Three failures on the feature/v2.0 Verify run, two of them latent behind the lint job's step ordering: - cpplint (lint lane): two build/include_what_you_use errors on webserver_impl_dispatch.hpp. The header is a class-body fragment (#error-gated, included mid-class) so it cannot carry its own #include lines; suppress with NOLINT on the two anchor declarations, matching the existing suppression already in this file. - complexity gate (lint lane, never reached because cpplint failed first): fire_response_sent_gated had crept to CCN 11. The `server_gate || route_gate` predicate was spelled out twice; hoisting it into a named `user_gate` both reads better and drops one boolean operator, bringing the function back to CCN 10. Behaviour-preserving. - valgrind-drd lane timeout (90 min): route_table_concurrency and ws_registry_concurrency spin up to 20 threads hammering a single shared_mutex for a 30 s real-time window. helgrind/drd track a happens-before relation over every lock/atomic across every thread pair, so that workload explodes their internal state (the drd lane hung with no output for 88 minutes). memcheck, which tracks no thread ordering, ran the same binary in ~60 s. Add stress_window() / stress_threads() helpers in test_utils.hpp that shrink the run window (30 s -> 3 s) and per-role fan-out (-> 3) only when VALGRIND is set; a race detector needs only a short concurrent window to flag a missing synchronisation. Native and TSan lanes keep the full-size stress. Verified locally: cpplint clean, check-complexity/file-size/ warning-suppression gates pass, and make check is 113/113. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5140308 commit 1bb47bd

5 files changed

Lines changed: 43 additions & 19 deletions

File tree

src/detail/webserver_hook_firing.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,12 @@ void webserver_impl::fire_response_sent_gated(detail::modded_request* mr,
188188
auto* rtable = resource != nullptr ? resource->hook_table_raw_() : nullptr;
189189
const bool route_gate = rtable != nullptr &&
190190
rtable->any_hooks(hook_phase::response_sent);
191+
// Whether any user hook (server-wide or per-route) will observe this
192+
// firing. Distinct from the log_access alias slot, which fires
193+
// regardless but never reads ctx.elapsed (see the elapsed gate below).
194+
const bool user_gate = server_gate || route_gate;
191195

192-
if (!server_gate && !route_gate && !hooks_.has_log_access_alias()) return;
196+
if (!user_gate && !hooks_.has_log_access_alias()) return;
193197
// mr->response is null only if materialize_and_queue_response's
194198
// belt-and-suspenders fallback also failed; fire nothing rather than crash.
195199
if (!mr->response) return;
@@ -203,7 +207,7 @@ void webserver_impl::fire_response_sent_gated(detail::modded_request* mr,
203207
// slot fires, to avoid a gratuitous clock syscall on the common path.
204208
// NOTE: the alias body MUST NOT read ctx.elapsed; any future change
205209
// that needs elapsed in the alias must also remove this optimisation.
206-
const auto elapsed = (server_gate || route_gate)
210+
const auto elapsed = user_gate
207211
? std::chrono::duration_cast<std::chrono::nanoseconds>(
208212
std::chrono::steady_clock::now() - mr->start_time)
209213
: std::chrono::nanoseconds::zero();

src/httpserver/detail/webserver_impl_dispatch.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,12 @@ bool resolve_resource_for_request(modded_request* mr,
319319
// header. On handler-throw, routes through the safe internal-error
320320
// path.
321321
void dispatch_resource_handler(modded_request* mr,
322-
const std::shared_ptr<::httpserver::http_resource>& hrm);
322+
const std::shared_ptr<::httpserver::http_resource>& hrm); // NOLINT(build/include_what_you_use)
323323

324324
// Serialize an allowed-method set into the comma-separated value
325325
// expected by the HTTP `Allow:` header. Enum-declaration order:
326326
// GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, PATCH.
327-
std::string serialize_allow_methods(method_set allowed) const;
327+
std::string serialize_allow_methods(method_set allowed) const; // NOLINT(build/include_what_you_use)
328328

329329
// Final stage of finalize_answer: build an MHD_Response from
330330
// mr->response, decorate it, queue it on the connection. Handles

test/integ/route_table_concurrency.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ LT_BEGIN_AUTO_TEST(route_table_concurrency_suite,
7878
std::atomic<int> writer_ops{0};
7979
std::atomic<int> reader_ops{0};
8080

81-
constexpr int kWriters = 4;
82-
constexpr int kReaders = 16;
81+
const int kWriters = stress_threads(4);
82+
const int kReaders = stress_threads(16);
8383

8484
std::vector<std::thread> threads;
8585
threads.reserve(kWriters + kReaders);
@@ -127,8 +127,7 @@ LT_BEGIN_AUTO_TEST(route_table_concurrency_suite,
127127
// valgrind/TSan overhead. Using an operation count avoids fixed
128128
// wall-clock sleeps that inflate on loaded CI boxes.
129129
constexpr int kOpThreshold = 10000;
130-
auto deadline =
131-
std::chrono::steady_clock::now() + std::chrono::seconds(30);
130+
auto deadline = std::chrono::steady_clock::now() + stress_window();
132131
while ((writer_ops.load() < kOpThreshold || reader_ops.load() < kOpThreshold)
133132
&& std::chrono::steady_clock::now() < deadline) {
134133
std::this_thread::sleep_for(std::chrono::milliseconds(10));
@@ -173,8 +172,8 @@ LT_BEGIN_AUTO_TEST(route_table_concurrency_suite,
173172
std::atomic<int> writer_ops{0};
174173
std::atomic<int> reader_ops{0};
175174

176-
constexpr int kWriters = 4;
177-
constexpr int kReaders = 8;
175+
const int kWriters = stress_threads(4);
176+
const int kReaders = stress_threads(8);
178177

179178
std::vector<std::thread> threads;
180179
threads.reserve(kWriters + kReaders);
@@ -223,8 +222,7 @@ LT_BEGIN_AUTO_TEST(route_table_concurrency_suite,
223222
}
224223

225224
// Same baseline + valgrind-tolerant extension as Cycle I above.
226-
auto deadline2 =
227-
std::chrono::steady_clock::now() + std::chrono::seconds(30);
225+
auto deadline2 = std::chrono::steady_clock::now() + stress_window();
228226
std::this_thread::sleep_for(std::chrono::milliseconds(500));
229227
while ((writer_ops.load() == 0 || reader_ops.load() == 0)
230228
&& std::chrono::steady_clock::now() < deadline2) {

test/integ/test_utils.hpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#ifndef TEST_INTEG_TEST_UTILS_HPP_
2222
#define TEST_INTEG_TEST_UTILS_HPP_
2323

24+
#include <chrono>
2425
#include <cstdlib>
2526

2627
// Shared helpers for the integration test suite.
@@ -33,6 +34,27 @@
3334
// (route_table_concurrency.cpp, ws_registry_concurrency.cpp) use this to
3435
// relax ONLY their liveness assertions, never the race/crash gate.
3536
inline bool under_valgrind() { return std::getenv("VALGRIND") != nullptr; }
37+
38+
// Concurrency-stress sizing knobs (route_table_concurrency.cpp,
39+
// ws_registry_concurrency.cpp). helgrind/drd track a happens-before
40+
// relation over every lock and atomic across every pair of threads, so the
41+
// native-size stress (up to 20 threads hammering a single shared_mutex for a
42+
// 30 s real-time window) makes the tool's internal segment/vector-clock
43+
// state grow superlinearly -- enough to blow the 90-minute CI budget (the
44+
// drd lane was observed hanging inside route_table_concurrency, producing no
45+
// output for 88 minutes). memcheck, which tracks no thread ordering, runs the
46+
// same binary in ~60 s. A race detector needs only a short window of genuine
47+
// concurrency to flag a missing-synchronisation access, so shrink BOTH the
48+
// per-role thread fan-out and the run window sharply under valgrind. The
49+
// native and TSan lanes (VALGRIND unset) keep the full-size stress.
50+
inline std::chrono::seconds stress_window() {
51+
return under_valgrind() ? std::chrono::seconds(3)
52+
: std::chrono::seconds(30);
53+
}
54+
inline int stress_threads(int full_native) {
55+
return under_valgrind() ? (full_native < 3 ? full_native : 3)
56+
: full_native;
57+
}
3658
//
3759
// Historical note: this header previously exposed `as_shared(http_resource&)`,
3860
// a thin wrapper that returned a std::shared_ptr<http_resource> with a no-op

test/integ/ws_registry_concurrency.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ LT_BEGIN_AUTO_TEST(ws_registry_concurrency_suite,
8888
std::atomic<int> writer_ops{0};
8989
std::atomic<int> reader_ops{0};
9090

91-
constexpr int kWriters = 4;
92-
constexpr int kReaders = 16;
91+
const int kWriters = stress_threads(4);
92+
const int kReaders = stress_threads(16);
9393

9494
std::vector<std::thread> threads;
9595
threads.reserve(kWriters + kReaders);
@@ -130,12 +130,12 @@ LT_BEGIN_AUTO_TEST(ws_registry_concurrency_suite,
130130
}
131131

132132
// Run until enough operations have been observed by both roles, or
133-
// until a wall-clock safety ceiling (30s) to survive valgrind/TSan
134-
// overhead. Mirrors route_table_concurrency.cpp's threshold-based
135-
// stop condition (avoids a fixed wall-clock sleep).
133+
// until stress_window() elapses (30s native/TSan; 3s under valgrind so
134+
// the helgrind/drd happens-before engine fits the CI budget). Mirrors
135+
// route_table_concurrency.cpp's threshold-based stop condition (avoids a
136+
// fixed wall-clock sleep).
136137
constexpr int kOpThreshold = 10000;
137-
auto deadline =
138-
std::chrono::steady_clock::now() + std::chrono::seconds(30);
138+
auto deadline = std::chrono::steady_clock::now() + stress_window();
139139
while ((writer_ops.load() < kOpThreshold || reader_ops.load() < kOpThreshold)
140140
&& std::chrono::steady_clock::now() < deadline) {
141141
std::this_thread::sleep_for(std::chrono::milliseconds(10));

0 commit comments

Comments
 (0)