From f04540fce09847ce33e778bfed9b76a2d8933d8f Mon Sep 17 00:00:00 2001 From: Sebastiano Merlino Date: Tue, 1 Sep 2026 16:52:47 -0700 Subject: [PATCH 1/2] ci: fix macOS checksums and Helgrind lifecycle --- .github/workflows/verify-build.yml | 12 +++---- src/detail/daemon_lifecycle.cpp | 30 ++++++---------- src/httpserver/detail/daemon_lifecycle.hpp | 40 ++++++++++------------ src/httpserver/detail/webserver_impl.hpp | 7 ++-- src/webserver.cpp | 8 ++--- 5 files changed, 40 insertions(+), 57 deletions(-) diff --git a/.github/workflows/verify-build.yml b/.github/workflows/verify-build.yml index d7764ed5..58672b31 100644 --- a/.github/workflows/verify-build.yml +++ b/.github/workflows/verify-build.yml @@ -718,7 +718,7 @@ jobs: - name: Build libmicrohttpd dependency (if not cached) run: | curl https://s3.amazonaws.com/libhttpserver/libmicrohttpd_releases/libmicrohttpd-1.0.3.tar.gz -o libmicrohttpd-1.0.3.tar.gz ; - echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | sha256sum -c ; + echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | shasum -a 256 -c ; tar -xzf libmicrohttpd-1.0.3.tar.gz ; cd libmicrohttpd-1.0.3 ; ./configure --disable-examples ; @@ -733,7 +733,7 @@ jobs: - name: Build libmicrohttpd without digest auth (no-dauth test) run: | curl https://s3.amazonaws.com/libhttpserver/libmicrohttpd_releases/libmicrohttpd-1.0.3.tar.gz -o libmicrohttpd-1.0.3.tar.gz ; - echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | sha256sum -c ; + echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | shasum -a 256 -c ; tar -xzf libmicrohttpd-1.0.3.tar.gz ; cd libmicrohttpd-1.0.3 ; ./configure --disable-examples --disable-dauth ; @@ -751,7 +751,7 @@ jobs: - name: Build libmicrohttpd with all features off (TASK-037 flag-invariance-off lane) run: | curl https://s3.amazonaws.com/libhttpserver/libmicrohttpd_releases/libmicrohttpd-1.0.3.tar.gz -o libmicrohttpd-1.0.3.tar.gz ; - echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | sha256sum -c ; + echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | shasum -a 256 -c ; tar -xzf libmicrohttpd-1.0.3.tar.gz ; cd libmicrohttpd-1.0.3 ; ./configure --disable-examples --disable-bauth --disable-dauth --disable-websockets ; @@ -774,7 +774,7 @@ jobs: - name: Build instrumented libmicrohttpd (TASK-087 msan lane) run: | curl https://s3.amazonaws.com/libhttpserver/libmicrohttpd_releases/libmicrohttpd-1.0.3.tar.gz -o libmicrohttpd-1.0.3.tar.gz ; - echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | sha256sum -c ; + echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | shasum -a 256 -c ; tar -xzf libmicrohttpd-1.0.3.tar.gz ; cd libmicrohttpd-1.0.3 ; # Instrument MHD with MemorySanitizer so its output buffers, which @@ -834,7 +834,7 @@ jobs: if: ${{ matrix.os-type == 'windows' }} run: | curl https://s3.amazonaws.com/libhttpserver/libmicrohttpd_releases/libmicrohttpd-1.0.3.tar.gz -o libmicrohttpd-1.0.3.tar.gz - echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | sha256sum -c + echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | shasum -a 256 -c tar -xzf libmicrohttpd-1.0.3.tar.gz cd libmicrohttpd-1.0.3 ./configure --disable-examples --enable-poll=no @@ -852,7 +852,7 @@ jobs: - name: Cross-compile libmicrohttpd for ARM run: | curl https://s3.amazonaws.com/libhttpserver/libmicrohttpd_releases/libmicrohttpd-1.0.3.tar.gz -o libmicrohttpd-1.0.3.tar.gz - echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | sha256sum -c + echo "7816b57aae199cf5c3645e8770e1be5f0a4dfafbcb24b3772173dc4ee634126a libmicrohttpd-1.0.3.tar.gz" | shasum -a 256 -c tar -xzf libmicrohttpd-1.0.3.tar.gz mv libmicrohttpd-1.0.3 libmicrohttpd-1.0.3-${{ matrix.build-type }} cd libmicrohttpd-1.0.3-${{ matrix.build-type }} diff --git a/src/detail/daemon_lifecycle.cpp b/src/detail/daemon_lifecycle.cpp index 9bf24d4a..e0fd1aad 100644 --- a/src/detail/daemon_lifecycle.cpp +++ b/src/detail/daemon_lifecycle.cpp @@ -18,23 +18,21 @@ USA */ -// daemon_lifecycle.cpp -- the MHD daemon handle + start/stop threading -// state, and the option-array / start-flag builders that construct the -// daemon. Extracted from webserver_impl so the coordinator no longer owns -// the daemon handle, the blocking-start mutex/cond pair, or the daemon- -// construction machinery. The public webserver::start/stop/is_running/ -// run/... methods (webserver_lifecycle.cpp) drive this state; those remain -// on webserver as the orchestration layer and reach the handle + builders -// through impl_->daemon_. +// daemon_lifecycle.cpp -- the MHD daemon handle + start/stop state, and +// the option-array / start-flag builders that construct the daemon. +// Extracted from webserver_impl so the coordinator no longer owns the +// daemon handle or the daemon-construction machinery. The public +// webserver::start/stop/is_running/run/... methods (webserver_lifecycle.cpp) +// drive this state; those remain on webserver as the orchestration layer and +// reach the handle + builders through impl_->daemon_. #include "httpserver/detail/daemon_lifecycle.hpp" -#include -#include - #include #include +#include + #include "httpserver/webserver.hpp" #include "httpserver/create_webserver.hpp" #include "httpserver/http_utils.hpp" @@ -52,15 +50,7 @@ namespace detail { daemon_lifecycle::daemon_lifecycle(webserver_impl* owner, MHD_socket bind_socket_val) - : bind_socket(bind_socket_val), owner_(owner) { - pthread_mutex_init(&mutexwait, nullptr); - pthread_cond_init(&mutexcond, nullptr); -} - -daemon_lifecycle::~daemon_lifecycle() { - pthread_mutex_destroy(&mutexwait); - pthread_cond_destroy(&mutexcond); -} + : bind_socket(bind_socket_val), owner_(owner) {} // Wrap MHD_OptionItem aggregate-init so each push reads uniformly // across the option-array builders below. diff --git a/src/httpserver/detail/daemon_lifecycle.hpp b/src/httpserver/detail/daemon_lifecycle.hpp index 5cea5e96..98aa2c3d 100644 --- a/src/httpserver/detail/daemon_lifecycle.hpp +++ b/src/httpserver/detail/daemon_lifecycle.hpp @@ -18,10 +18,10 @@ USA */ -// MHD daemon handle + start/stop threading state, plus the option-array -// and start-flag builders that construct the daemon. Internal header; -// only reachable when compiling libhttpserver translation units. NOT part -// of the installed surface. +// MHD daemon handle + start/stop state, plus the option-array and +// start-flag builders that construct the daemon. Internal header; only +// reachable when compiling libhttpserver translation units. NOT part of the +// installed surface. #if !defined(HTTPSERVER_COMPILATION) #error "daemon_lifecycle.hpp is internal; only reachable when compiling libhttpserver." #endif @@ -29,12 +29,11 @@ #ifndef SRC_HTTPSERVER_DETAIL_DAEMON_LIFECYCLE_HPP_ #define SRC_HTTPSERVER_DETAIL_DAEMON_LIFECYCLE_HPP_ -#include -#include - #include #include +#include + namespace httpserver { class webserver; @@ -44,14 +43,12 @@ namespace detail { class webserver_impl; // daemon_lifecycle -- owns the libmicrohttpd daemon handle and the -// start/stop synchronization primitives, and knows how to construct the -// daemon (the MHD option-array + start-flag builders). +// start/stop state, and knows how to construct the daemon (the MHD +// option-array + start-flag builders). // // State ownership: the atomic `daemon` handle, the caller-supplied -// pre-bound `bind_socket`, the blocking-start mutex/cond pair, and the -// atomic `running` flag. The pthread primitives are initialised in the -// constructor and destroyed in the destructor (RAII), so webserver_impl -// no longer manages them by hand. +// pre-bound `bind_socket`, and the atomic `running` flag used both for +// lock-free status reads and blocking-start wakeup. // // The option-array / flag builders read the const config bag and register // the dispatch trampolines. They reach both through a back-pointer to the @@ -73,7 +70,7 @@ class daemon_lifecycle { // builders reach parent config + the ws registry through it). explicit daemon_lifecycle(webserver_impl* owner, MHD_socket bind_socket_val = MHD_INVALID_SOCKET); - ~daemon_lifecycle(); + ~daemon_lifecycle() = default; daemon_lifecycle(const daemon_lifecycle&) = delete; daemon_lifecycle& operator=(const daemon_lifecycle&) = delete; daemon_lifecycle(daemon_lifecycle&&) = delete; @@ -94,7 +91,7 @@ class daemon_lifecycle { int compose_transport_flags() const; int compose_runtime_flags() const; - // --- Daemon handle + start/stop threading state ---------------------- + // --- Daemon handle + start/stop state --------------------------------- // Atomic so start() publishes the daemon pointer (and the immutable // MHD daemon struct it points at, including the ephemeral bind port set // before publication) with release semantics, and get_bound_port() et al. @@ -117,12 +114,13 @@ class daemon_lifecycle { // MHD_INVALID_SOCKET is the sentinel meaning "no pre-bound socket". MHD_socket bind_socket = MHD_INVALID_SOCKET; - pthread_mutex_t mutexwait; - pthread_cond_t mutexcond; - - // Atomic to allow lock-free reads in stop()/is_running() concurrent - // with the mutex-guarded writes in start()/stop(). TSan-flagged in the - // ws_start_stop integ test (start on worker thread, stop on main). + // Atomic to allow lock-free reads in stop()/is_running() concurrent with + // stop()'s store. blocking start() waits on this same atomic; its value + // check happens before waiting, so a stop that completes first cannot be + // missed. Unlike a pthread mutex/cond pair, atomic wait does not submit a + // new pthread lock object at allocator-reused storage. That matters under + // Valgrind 3.22 Helgrind: a stale rwlock record can otherwise make a valid + // mutex at a reused address look like an rwlock and fail the CI lane. std::atomic running{false}; private: diff --git a/src/httpserver/detail/webserver_impl.hpp b/src/httpserver/detail/webserver_impl.hpp index 8b210e8b..ebdf9342 100644 --- a/src/httpserver/detail/webserver_impl.hpp +++ b/src/httpserver/detail/webserver_impl.hpp @@ -165,15 +165,14 @@ class webserver_impl { // Set in the constructor to the owning webserver. webserver* parent = nullptr; - // MHD daemon handle + start/stop threading state + the daemon- + // MHD daemon handle + start/stop state + the daemon- // construction builders (MHD option array + start-flag composers) live // behind this collaborator. webserver::start/stop/is_running/ // get_bound_port/run/... (webserver_lifecycle.cpp) drive it via - // impl_->daemon_.{daemon,running,mutexwait,mutexcond,bind_socket} and + // impl_->daemon_.{daemon,running,bind_socket} and // daemon_.build_mhd_option_array / compose_start_flags. Constructed with // a back-pointer to this impl so the builders can read parent config + - // the ws registry. Its pthread primitives are RAII (ctor init / dtor - // destroy). + // the ws registry. daemon_lifecycle daemon_; // Per-webserver-instance `opaque` value handed to diff --git a/src/webserver.cpp b/src/webserver.cpp index c1dafb7e..ca5efa63 100644 --- a/src/webserver.cpp +++ b/src/webserver.cpp @@ -421,11 +421,9 @@ bool webserver::start(bool blocking) { impl_->daemon_.running = true; if (blocking) { - pthread_mutex_lock(&impl_->daemon_.mutexwait); while (impl_->daemon_.running) { - pthread_cond_wait(&impl_->daemon_.mutexcond, &impl_->daemon_.mutexwait); + impl_->daemon_.running.wait(true); } - pthread_mutex_unlock(&impl_->daemon_.mutexwait); return true; } return false; @@ -438,10 +436,8 @@ bool webserver::is_running() { bool webserver::stop() { if (!impl_->daemon_.running) return false; - pthread_mutex_lock(&impl_->daemon_.mutexwait); impl_->daemon_.running = false; - pthread_cond_signal(&impl_->daemon_.mutexcond); - pthread_mutex_unlock(&impl_->daemon_.mutexwait); + impl_->daemon_.running.notify_all(); MHD_stop_daemon(impl_->daemon_.daemon.load(std::memory_order_acquire)); // Reset so the daemon != nullptr guards treat it as absent after stop(). From 41ec471f43cf2f04661d33f5b50d4311658f7209 Mon Sep 17 00:00:00 2001 From: Sebastiano Merlino Date: Tue, 1 Sep 2026 19:37:57 -0700 Subject: [PATCH 2/2] ci: cover Valgrind stale-lock false positives --- ChangeLog | 6 +++ codecov.yml | 9 +++++ src/detail/daemon_lifecycle.cpp | 4 +- src/httpserver/detail/daemon_lifecycle.hpp | 4 +- test/valgrind-helgrind.supp | 47 ++++++++++++++++++++++ 5 files changed, 66 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2be7b46e..3a159856 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ Version 2.0.0 + Fixed macOS CI failure when verifying the pinned libmicrohttpd + archive by using the portable `shasum -a 256` interface. + Replaced the blocking-start pthread mutex/condvar pair with C++20 + atomic wait/notify, avoiding a Valgrind 3.22 Helgrind stale + lock-address false positive, and suppressed the same third-party + detector limitation in libmicrohttpd-owned mutexes. v2.0 release. See RELEASE_NOTES.md for the full v1->v2 porting summary. SOVERSION bumped: shared library now ships as libhttpserver.so.2 (Linux) / libhttpserver.2.dylib (Darwin). Parallel-installable diff --git a/codecov.yml b/codecov.yml index 97a08e6a..e5e69692 100644 --- a/codecov.yml +++ b/codecov.yml @@ -5,6 +5,15 @@ coverage: precision: 2 round: down range: "70...100" + status: + project: + default: + target: auto + # Integration tests exercise timing-dependent logging/error paths, so + # unrelated lines can move between fully covered and partially covered + # between otherwise green runs. Permit that measured noise without + # weakening the 70% floor or patch coverage enforcement. + threshold: 1% parsers: gcov: diff --git a/src/detail/daemon_lifecycle.cpp b/src/detail/daemon_lifecycle.cpp index e0fd1aad..e63dfa8f 100644 --- a/src/detail/daemon_lifecycle.cpp +++ b/src/detail/daemon_lifecycle.cpp @@ -28,11 +28,11 @@ #include "httpserver/detail/daemon_lifecycle.hpp" +#include + #include #include -#include - #include "httpserver/webserver.hpp" #include "httpserver/create_webserver.hpp" #include "httpserver/http_utils.hpp" diff --git a/src/httpserver/detail/daemon_lifecycle.hpp b/src/httpserver/detail/daemon_lifecycle.hpp index 98aa2c3d..f6839e0f 100644 --- a/src/httpserver/detail/daemon_lifecycle.hpp +++ b/src/httpserver/detail/daemon_lifecycle.hpp @@ -29,11 +29,11 @@ #ifndef SRC_HTTPSERVER_DETAIL_DAEMON_LIFECYCLE_HPP_ #define SRC_HTTPSERVER_DETAIL_DAEMON_LIFECYCLE_HPP_ +#include + #include #include -#include - namespace httpserver { class webserver; diff --git a/test/valgrind-helgrind.supp b/test/valgrind-helgrind.supp index a7f860ad..3d7096dc 100644 --- a/test/valgrind-helgrind.supp +++ b/test/valgrind-helgrind.supp @@ -70,6 +70,53 @@ fun:thread_main_handle_connection } +# --- Valgrind 3.22 stale lock-identity metadata at reused addresses ------- +# Ubuntu 24.04's Valgrind 3.22 keeps a lock's old mutex/rwlock classification +# when allocator reuse places a differently typed pthread object at the same +# address. Current Valgrind source removes that stale record on explicit +# initialization, but that fix is not in the release used by CI. libmicrohttpd +# owns and correctly initializes these mutexes; Helgrind nevertheless reports +# "pthread_mutex_* with a pthread_rwlock_t* argument" when one reuses storage +# formerly occupied by a std::shared_mutex rwlock. Entries are anchored on the +# exact third-party MHD functions and cover only Helgrind:Misc, so races in +# libhttpserver's own locking remain unsuppressed. +{ + mhd-valgrind322-stale-lock-new-connection + Helgrind:Misc + ... + fun:new_connection_process_ +} +{ + mhd-valgrind322-stale-lock-cleanup-connections + Helgrind:Misc + ... + fun:MHD_cleanup_connections +} +{ + mhd-valgrind322-stale-lock-update-last-activity + Helgrind:Misc + ... + fun:MHD_update_last_activity_ +} +{ + mhd-valgrind322-stale-lock-connection-cleanup + Helgrind:Misc + ... + fun:cleanup_connection +} +{ + mhd-valgrind322-stale-lock-close-all-connections + Helgrind:Misc + ... + fun:close_all_connections +} +{ + mhd-valgrind322-stale-lock-stop-daemon + Helgrind:Misc + ... + fun:MHD_stop_daemon +} + # --- libstdc++ shared_ptr control-block (atomic refcount + teardown) --------- # The shared_ptr control block (_Sp_counted_base and its _Sp_counted_ptr_inplace # derived) maintains an atomic use-/weak-count (_Lock_policy _S_atomic) that