Skip to content

Commit b596fc8

Browse files
etrclaude
andcommitted
refactor: extract ws_registry collaborator from webserver_impl
Second step of the webserver_impl decomposition. The URL -> websocket_handler map and its mutex were another independent state cluster on the god-object. Move them behind detail::ws_registry (try_register / unregister / find / empty). webserver_impl holds it as 'ws_registry ws_;' (HAVE_WEBSOCKET-gated). register/unregister_ws_resource mutate it; complete_websocket_upgrade resolves a handler via find() — which returns a shared_ptr copy under the read lock, preserving the keep-alive-across-upgrade guarantee; compose_transport_flags consults empty() for MHD_ALLOW_UPGRADE. The registry never dereferences websocket_handler (only stores/erases/copies shared_ptrs), so its TU compiles feature-independently. ws_upgrade_data/upgrade_handler stay as dispatch glue for now (they move with the dispatch pipeline). No behavioural change; build-green, 109/109. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c70b4d9 commit b596fc8

7 files changed

Lines changed: 155 additions & 31 deletions

File tree

src/Makefile.am

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ lib_LTLIBRARIES = libhttpserver.la
2525
# builds. The WS-off branch in websocket_handler.cpp provides stub
2626
# definitions (every member throws feature_unavailable except is_valid()
2727
# which returns false).
28-
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp webserver_add_hook.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_response_factories.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp cookie.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/ip_access_control.cpp detail/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_routes_upsert.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_response_queue.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_hook_firing.cpp detail/hook_phase_dispatch.cpp
28+
libhttpserver_la_SOURCES = string_utilities.cpp webserver.cpp webserver_add_hook.cpp http_utils.cpp file_info.cpp http_request.cpp http_request_auth.cpp http_response.cpp http_response_factories.cpp http_resource.cpp create_webserver.cpp create_test_request.cpp websocket_handler.cpp hook_handle.cpp peer_address.cpp resource_hook_table.cpp cookie.cpp detail/http_endpoint.cpp detail/body.cpp detail/ip_representation.cpp detail/ip_access_control.cpp detail/ws_registry.cpp detail/http_request_impl.cpp detail/http_request_impl_args.cpp detail/http_request_impl_tls.cpp detail/webserver_lifecycle.cpp detail/webserver_register.cpp detail/webserver_routes.cpp detail/webserver_routes_upsert.cpp detail/webserver_callbacks.cpp detail/webserver_callbacks_lifecycle.cpp detail/webserver_websocket.cpp detail/webserver_dispatch.cpp detail/webserver_request.cpp detail/webserver_response_queue.cpp detail/webserver_body_pipeline.cpp detail/webserver_error_pages.cpp detail/webserver_aliases.cpp detail/webserver_hook_firing.cpp detail/hook_phase_dispatch.cpp
2929
# noinst_HEADERS: shipped in the tarball but NEVER installed under $prefix/include.
3030
# Detail headers (httpserver/detail/*.hpp) live here so they cannot leak to
3131
# downstream consumers — the public surface comes in through <httpserver.hpp>.
32-
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/webserver_impl_dispatch.hpp httpserver/detail/connection_state.hpp httpserver/detail/ip_access_control.hpp httpserver/detail/secure_zero.hpp httpserver/detail/http_request_impl.hpp httpserver/detail/resource_hook_table.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/segment_trie.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp httpserver/detail/unescape_helpers.hpp gettext.h
32+
noinst_HEADERS = httpserver/string_utilities.hpp httpserver/detail/modded_request.hpp httpserver/detail/http_endpoint.hpp httpserver/detail/body.hpp httpserver/detail/webserver_impl.hpp httpserver/detail/webserver_impl_dispatch.hpp httpserver/detail/connection_state.hpp httpserver/detail/ip_access_control.hpp httpserver/detail/ws_registry.hpp httpserver/detail/secure_zero.hpp httpserver/detail/http_request_impl.hpp httpserver/detail/resource_hook_table.hpp httpserver/detail/route_entry.hpp httpserver/detail/lambda_resource.hpp httpserver/detail/segment_trie.hpp httpserver/detail/route_cache.hpp httpserver/detail/route_tier.hpp httpserver/detail/unescape_helpers.hpp gettext.h
3333
nobase_include_HEADERS = httpserver.hpp httpserver/body_kind.hpp httpserver/cookie.hpp httpserver/constants.hpp httpserver/create_webserver.hpp httpserver/create_webserver_setters.hpp httpserver/create_test_request.hpp httpserver/webserver.hpp httpserver/webserver_routes.hpp httpserver/webserver_runtime.hpp httpserver/webserver_websocket.hpp httpserver/webserver_hooks.hpp httpserver/websocket_handler.hpp httpserver/http_utils.hpp httpserver/http_utils_helpers.hpp httpserver/ip_representation.hpp httpserver/file_info.hpp httpserver/http_request.hpp httpserver/http_response.hpp httpserver/http_resource.hpp httpserver/feature_unavailable.hpp httpserver/iovec_entry.hpp httpserver/http_arg_value.hpp httpserver/http_method.hpp httpserver/hook_phase.hpp httpserver/hook_action.hpp httpserver/hook_handle.hpp httpserver/hook_context.hpp
3434

3535
AM_CXXFLAGS += -fPIC -Wall

src/detail/webserver_lifecycle.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ int webserver_impl::compose_runtime_flags() const {
271271
if (parent->config.turbo) flags |= MHD_USE_TURBO;
272272
if (parent->config.suppress_date_header) flags |= MHD_USE_SUPPRESS_DATE_NO_CLOCK;
273273
#ifdef HAVE_WEBSOCKET
274-
if (!registered_ws_handlers.empty()) flags |= MHD_ALLOW_UPGRADE;
274+
if (!ws_.empty()) flags |= MHD_ALLOW_UPGRADE;
275275
#endif // HAVE_WEBSOCKET
276276
return flags;
277277
}

src/detail/webserver_routes.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,7 @@ void webserver::register_ws_resource(const std::string& resource,
248248
throw std::invalid_argument("The websocket_handler pointer cannot be null");
249249
}
250250
std::string url_key = http_utils::standardize_url(resource);
251-
std::unique_lock lock(impl_->registered_ws_handlers_mutex_);
252-
auto result = impl_->registered_ws_handlers.emplace(std::move(url_key),
253-
std::move(handler));
254-
if (!result.second) {
251+
if (!impl_->ws_.try_register(std::move(url_key), std::move(handler))) {
255252
// v1's operator[]-based insert silently overwrote; v2.0
256253
// surfaces the collision by throwing.
257254
throw std::invalid_argument(
@@ -268,8 +265,7 @@ void webserver::register_ws_resource(const std::string& resource,
268265

269266
void webserver::unregister_ws_resource(const std::string& resource) {
270267
#ifdef HAVE_WEBSOCKET
271-
std::unique_lock lock(impl_->registered_ws_handlers_mutex_);
272-
impl_->registered_ws_handlers.erase(http_utils::standardize_url(resource));
268+
impl_->ws_.unregister(http_utils::standardize_url(resource));
273269
#else
274270
(void)resource;
275271
throw feature_unavailable("websocket", "HAVE_WEBSOCKET");

src/detail/webserver_websocket.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,13 @@ std::optional<MHD_Result>
209209
webserver_impl::complete_websocket_upgrade(MHD_Connection* connection,
210210
detail::modded_request* mr,
211211
const char* ws_key) {
212-
std::shared_lock lock(registered_ws_handlers_mutex_);
213-
auto ws_it = registered_ws_handlers.find(mr->standardized_url);
214-
if (ws_it == registered_ws_handlers.end()) {
212+
// find() returns a shared_ptr copy taken under the registry's read lock,
213+
// so the handler is kept alive across the MHD upgrade callback even if
214+
// unregister_ws_resource erases the slot mid-upgrade.
215+
std::shared_ptr<websocket_handler> handler_sp = ws_.find(mr->standardized_url);
216+
if (!handler_sp) {
215217
return std::nullopt;
216218
}
217-
// Take a shared_ptr copy under the shared lock so the
218-
// handler is kept alive across the MHD upgrade callback even if
219-
// unregister_ws_resource erases the slot mid-upgrade.
220-
std::shared_ptr<websocket_handler> handler_sp = ws_it->second;
221-
lock.unlock();
222219

223220
// CWE-401: RAII guard so data is freed if MHD_create_response_for_upgrade
224221
// returns null. Ownership is transferred to MHD (via release()) only after

src/detail/ws_registry.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2026 Sebastiano Merlino
4+
5+
This library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
This library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with this library; if not, write to the Free Software
17+
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
18+
USA
19+
*/
20+
21+
#include "httpserver/detail/ws_registry.hpp"
22+
23+
#include <map>
24+
#include <memory>
25+
#include <mutex>
26+
#include <shared_mutex>
27+
#include <string>
28+
#include <utility>
29+
30+
namespace httpserver {
31+
namespace detail {
32+
33+
bool ws_registry::try_register(std::string url_key,
34+
std::shared_ptr<websocket_handler> handler) {
35+
std::unique_lock lock(mutex_);
36+
return handlers_.emplace(std::move(url_key), std::move(handler)).second;
37+
}
38+
39+
void ws_registry::unregister(const std::string& url_key) {
40+
std::unique_lock lock(mutex_);
41+
handlers_.erase(url_key);
42+
}
43+
44+
std::shared_ptr<websocket_handler> ws_registry::find(
45+
const std::string& url) const {
46+
std::shared_lock lock(mutex_);
47+
auto it = handlers_.find(url);
48+
return it == handlers_.end() ? nullptr : it->second;
49+
}
50+
51+
bool ws_registry::empty() const {
52+
std::shared_lock lock(mutex_);
53+
return handlers_.empty();
54+
}
55+
56+
} // namespace detail
57+
} // namespace httpserver

src/httpserver/detail/webserver_impl.hpp

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
#include "httpserver/detail/segment_trie.hpp"
7777
#include "httpserver/detail/route_cache.hpp"
7878
#include "httpserver/detail/route_entry.hpp"
79+
#include "httpserver/detail/ws_registry.hpp"
7980

8081
#if MHD_VERSION < 0x00097002
8182
typedef int MHD_Result;
@@ -261,7 +262,7 @@ class webserver_impl {
261262
// resolve_resource_for_request() calls it exclusively, and the
262263
// 3-tier table is the single routing surface. Lambda/class conflict
263264
// detection probes the same tiers (find_v2_entry_by_path_);
264-
// WebSocket dispatch uses a dedicated registered_ws_handlers_mutex_.
265+
// WebSocket dispatch resolves handlers through the separate ws_ registry.
265266
lookup_result lookup_v2(http_method method, const std::string& path);
266267

267268
// Lifecycle hook bus.
@@ -380,19 +381,14 @@ class webserver_impl {
380381
ip_access_control acl_;
381382

382383
#ifdef HAVE_WEBSOCKET
383-
// shared_ptr storage. The dispatch path
384-
// (complete_websocket_upgrade) takes a shared_ptr copy under the
385-
// shared lock that keeps the handler alive across an MHD upgrade
386-
// callback, even if unregister_ws_resource races to drop the
387-
// registration mid-upgrade. The webserver always holds one reference
388-
// until the slot is erased.
389-
//
390-
// Lock: registered_ws_handlers_mutex_ guards this map exclusively,
391-
// independent of the HTTP route_table_mutex_; no call site ever
392-
// holds both mutexes simultaneously.
393-
std::shared_mutex registered_ws_handlers_mutex_;
394-
std::map<std::string, std::shared_ptr<::httpserver::websocket_handler>>
395-
registered_ws_handlers;
384+
// WebSocket handler registry (URL -> handler map + its mutex) lives
385+
// behind this collaborator. register/unregister_ws_resource mutate it;
386+
// complete_websocket_upgrade resolves a handler via find() (taking a
387+
// shared_ptr copy that keeps the handler alive across the MHD upgrade
388+
// callback even if unregister races mid-upgrade); start() consults
389+
// empty() for MHD_ALLOW_UPGRADE. Its mutex is independent of every
390+
// other cluster's; no call site holds two of them at once.
391+
ws_registry ws_;
396392

397393
struct ws_upgrade_data {
398394
webserver_impl* impl;
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
This file is part of libhttpserver
3+
Copyright (C) 2011-2026 Sebastiano Merlino
4+
5+
This library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
This library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with this library; if not, write to the Free Software
17+
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
18+
USA
19+
*/
20+
21+
// WebSocket handler registry collaborator. Internal header; only reachable
22+
// when compiling libhttpserver translation units. NOT part of the installed
23+
// surface.
24+
#if !defined(HTTPSERVER_COMPILATION)
25+
#error "ws_registry.hpp is internal; only reachable when compiling libhttpserver."
26+
#endif
27+
28+
#ifndef SRC_HTTPSERVER_DETAIL_WS_REGISTRY_HPP_
29+
#define SRC_HTTPSERVER_DETAIL_WS_REGISTRY_HPP_
30+
31+
#include <map>
32+
#include <memory>
33+
#include <shared_mutex>
34+
#include <string>
35+
36+
namespace httpserver {
37+
38+
class websocket_handler;
39+
40+
namespace detail {
41+
42+
// Owns the URL -> websocket_handler map and its mutex — the whole of the
43+
// webserver's websocket-registration state, extracted from webserver_impl.
44+
// webserver::{register,unregister}_ws_resource mutate it; the dispatch-side
45+
// upgrade path resolves a handler via find(); start() consults empty() to
46+
// decide MHD_ALLOW_UPGRADE. This type never dereferences websocket_handler
47+
// (it only stores/erases/copies shared_ptrs), so it is feature-independent
48+
// and compiles even on HAVE_WEBSOCKET-off builds.
49+
class ws_registry {
50+
public:
51+
// Register @p handler at @p url_key. Returns false if a handler is
52+
// already present at that key (the caller surfaces the collision), true
53+
// on insert. Takes the write lock.
54+
bool try_register(std::string url_key,
55+
std::shared_ptr<websocket_handler> handler);
56+
57+
// Erase any handler registered at @p url_key. Takes the write lock.
58+
void unregister(const std::string& url_key);
59+
60+
// Return a shared_ptr copy of the handler registered at @p url, or
61+
// nullptr if none. The copy keeps the handler alive across an MHD
62+
// upgrade even if a concurrent unregister races to drop the slot.
63+
// Takes the read lock.
64+
[[nodiscard]] std::shared_ptr<websocket_handler> find(
65+
const std::string& url) const;
66+
67+
// True iff no handler is registered. Takes the read lock.
68+
[[nodiscard]] bool empty() const;
69+
70+
private:
71+
mutable std::shared_mutex mutex_;
72+
std::map<std::string, std::shared_ptr<websocket_handler>> handlers_;
73+
};
74+
75+
} // namespace detail
76+
} // namespace httpserver
77+
78+
#endif // SRC_HTTPSERVER_DETAIL_WS_REGISTRY_HPP_

0 commit comments

Comments
 (0)