Skip to content

Commit e9cf580

Browse files
committed
fix: make httplib handlers more defensive during shutdown
Additional crash fixes for write_response_core SIGSEGV: 1. Exception handler now captures m_stopping by pointer and m_logger by shared_ptr instead of capturing 'this'. This prevents accessing invalid memory if the exception handler is called during shutdown. 2. Exception handler checks m_stopping flag first and returns early with 503 if shutting down, avoiding any access to potentially invalid state. 3. Route handlers now check m_stopping multiple times: - At entry before any work - After acquiring mutex (shutdown may have started while waiting) - Before setting response content 4. Logging is now guarded by m_stopping checks to avoid accessing the logger after it may have been invalidated. The crash was occurring in write_response_core with a corrupted pointer (0x0000002d000000ac), suggesting use-after-free of a std::function or map during response writing. These changes minimize the window where handlers can access Impl state during shutdown.
1 parent 11f49f0 commit e9cf580

1 file changed

Lines changed: 66 additions & 10 deletions

File tree

src/odr/http_server.cpp

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,31 @@ class HttpServer::Impl {
2121
m_server{std::make_unique<httplib::Server>()} {
2222
// Set up exception handler to catch any internal httplib exceptions.
2323
// This prevents crashes when exceptions occur during request processing.
24+
// NOTE: We capture m_stopping by pointer (not 'this') to safely check
25+
// if we're shutting down before accessing other members.
2426
m_server->set_exception_handler(
25-
[this](const httplib::Request & /*req*/, httplib::Response &res,
26-
std::exception_ptr ep) {
27+
[stopping = &m_stopping, logger = m_logger](
28+
const httplib::Request & /*req*/, httplib::Response &res,
29+
std::exception_ptr ep) {
30+
// Check stopping flag before accessing any other state.
31+
// During shutdown, just set error status without logging.
32+
if (stopping->load(std::memory_order_acquire)) {
33+
res.status = 503;
34+
res.set_content("Service Unavailable", "text/plain");
35+
return;
36+
}
2737
try {
2838
if (ep) {
2939
std::rethrow_exception(ep);
3040
}
3141
} catch (const std::exception &e) {
32-
ODR_ERROR(*m_logger, "Exception in HTTP handler: " << e.what());
42+
if (logger) {
43+
ODR_ERROR(*logger, "Exception in HTTP handler: " << e.what());
44+
}
3345
} catch (...) {
34-
ODR_ERROR(*m_logger, "Unknown exception in HTTP handler");
46+
if (logger) {
47+
ODR_ERROR(*logger, "Unknown exception in HTTP handler");
48+
}
3549
}
3650
res.status = 500;
3751
res.set_content("Internal Server Error", "text/plain");
@@ -90,6 +104,13 @@ class HttpServer::Impl {
90104

91105
void serve_file(const httplib::Request &req, httplib::Response &res) {
92106
try {
107+
// Check stopping flag at entry - return immediately if shutting down
108+
if (m_stopping.load(std::memory_order_acquire)) {
109+
res.status = 503;
110+
res.set_content("Service Unavailable", "text/plain");
111+
return;
112+
}
113+
93114
std::string id = req.matches[1].str();
94115
std::string path = req.matches.size() > 1 ? req.matches[2].str() : "";
95116

@@ -103,27 +124,50 @@ class HttpServer::Impl {
103124
auto [_, service] = it->second;
104125
lock.unlock();
105126

127+
// Check again after lock - shutdown may have started while waiting
128+
if (m_stopping.load(std::memory_order_acquire)) {
129+
res.status = 503;
130+
res.set_content("Service Unavailable", "text/plain");
131+
return;
132+
}
133+
106134
serve_file(res, service, path);
107135
} catch (const std::exception &e) {
108-
ODR_ERROR(*m_logger, "Error handling request: " << e.what());
136+
// Don't log if we're shutting down - logger may be invalid
137+
if (!m_stopping.load(std::memory_order_acquire)) {
138+
ODR_ERROR(*m_logger, "Error handling request: " << e.what());
139+
}
109140
res.status = 500;
110141
res.set_content("Internal Server Error", "text/plain");
111142
} catch (...) {
112-
ODR_ERROR(*m_logger, "Unknown error handling request");
143+
if (!m_stopping.load(std::memory_order_acquire)) {
144+
ODR_ERROR(*m_logger, "Unknown error handling request");
145+
}
113146
res.status = 500;
114147
res.set_content("Internal Server Error", "text/plain");
115148
}
116149
}
117150

118151
void serve_file(httplib::Response &res, const HtmlService &service,
119152
const std::string &path) const {
153+
// Check stopping flag - return early if shutting down
154+
if (m_stopping.load(std::memory_order_acquire)) {
155+
res.status = 503;
156+
res.set_content("Service Unavailable", "text/plain");
157+
return;
158+
}
159+
120160
if (!service.exists(path)) {
121-
ODR_ERROR(*m_logger, "File not found: " << path);
161+
if (!m_stopping.load(std::memory_order_acquire)) {
162+
ODR_ERROR(*m_logger, "File not found: " << path);
163+
}
122164
res.status = 404;
123165
return;
124166
}
125167

126-
ODR_VERBOSE(*m_logger, "Serving file: " << path);
168+
if (!m_stopping.load(std::memory_order_acquire)) {
169+
ODR_VERBOSE(*m_logger, "Serving file: " << path);
170+
}
127171

128172
// Buffer content to avoid streaming issues on Android.
129173
// Using ContentProviderWithoutLength (chunked transfer encoding) can cause
@@ -136,13 +180,25 @@ class HttpServer::Impl {
136180
try {
137181
std::ostringstream buffer;
138182
service.write(path, buffer);
183+
184+
// Final check before setting content
185+
if (m_stopping.load(std::memory_order_acquire)) {
186+
res.status = 503;
187+
res.set_content("Service Unavailable", "text/plain");
188+
return;
189+
}
190+
139191
res.set_content(buffer.str(), service.mimetype(path));
140192
} catch (const std::exception &e) {
141-
ODR_ERROR(*m_logger, "Error serving file " << path << ": " << e.what());
193+
if (!m_stopping.load(std::memory_order_acquire)) {
194+
ODR_ERROR(*m_logger, "Error serving file " << path << ": " << e.what());
195+
}
142196
res.status = 500;
143197
res.set_content("Internal Server Error", "text/plain");
144198
} catch (...) {
145-
ODR_ERROR(*m_logger, "Unknown error serving file: " << path);
199+
if (!m_stopping.load(std::memory_order_acquire)) {
200+
ODR_ERROR(*m_logger, "Unknown error serving file: " << path);
201+
}
146202
res.status = 500;
147203
res.set_content("Internal Server Error", "text/plain");
148204
}

0 commit comments

Comments
 (0)