uNetworking / uNetworking/uWebSockets
Double-Free of UserData when gracefully closing a Websocket from Server-Side
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 19k
- Forks
- 1.9k
- Avg merge
- 3h 24m
- Merged PRs (30d)
- 5
Description
Hi,
i am trying to implement a graceful server Shutdown. Either i am doing something wrong, or there is a fundamental bug in the implementation. ASAN complains (rightfully) about a double-free attempt.
My Shutdown-Code involves:
loop->defer([this]
{
for (auto* con: websocketConnections)
con->end(1000, "Server shutting down");
if (appPtr)
appPtr->close();
});
The code of con->end() currently finishes with the following lines:
/* Emit close event */
if (webSocketContextData->closeHandler) {
webSocketContextData->closeHandler(this, code, message);
}
((USERDATA *) this->getUserData())->~USERDATA();
As we can see, the destructor of UserData is called at the very end.
However webSocketContextData->closeHandler() already brings me into uWS::TemplatedApp::ws in App.hpp, line 263 - 439.
Within this function there is this snippet:
/* Copy all handlers */
webSocketContext->getExt()->openHandler = std::move(behavior.open);
webSocketContext->getExt()->messageHandler = std::move(behavior.message);
webSocketContext->getExt()->drainHandler = std::move(behavior.drain);
webSocketContext->getExt()->subscriptionHandler = std::move(behavior.subscription);
webSocketContext->getExt()->closeHandler = std::move([closeHandler = std::move(behavior.close)](WebSocket<SSL, true, UserData> *ws, int code, std::string_view message) mutable {
if (closeHandler) {
closeHandler(ws, code, message);
}
/* Destruct user data after returning from close handler */
((UserData *) ws->getUserData())->~UserData(); // <<<<<<<<<<<<<<<<<<<<<<<<<-----
});
webSocketContext->getExt()->pingHandler = std::move(behavior.ping);
webSocketContext->getExt()->pongHandler = std::move(behavior.pong);
Where i have marked the destructor-call.
So, i am running into a code-path where the destructor is definitely called twice.
This is the full call-stack
[nelson] <lambda#1>::operator()(uWS::WebSocket<true, true, WebsocketData> *, int, std::string_view) App.h:383 <----------------- is calling the destructor
[nelson] std::__invoke_impl<void, <lambda#1> &, uWS::WebSocket<true, true, WebsocketData> *, int, std::string_view>(std::__invoke_other, <lambda#1> &, uWS::WebSocket<true, true, WebsocketData> *&&, int &&, std::string_view &&) invoke.h:63
[nelson] std::__invoke_r<void, <lambda#1> &, uWS::WebSocket<true, true, WebsocketData> *, int, std::string_view>(<lambda#1> &, uWS::WebSocket<true, true, WebsocketData> *&&, int &&, std::string_view &&) invoke.h:113
[nelson] std::move_only_function<void (uWS::WebSocket<true, true, WebsocketData> *, int, std::string_view)>::_S_invoke<<lambda#1> >(std::_Mofunc_base *, uWS::WebSocket<true, true, WebsocketData> *, int, std::string_view &&) mofunc_impl.h:219
[nelson] std::move_only_function<void (uWS::WebSocket<true, true, WebsocketData> *, int, std::string_view)>::operator() mofunc_impl.h:184
[nelson] uWS::WebSocket<true, true, WebsocketData>::end WebSocket.h:266 <----------------- will call destructor again once this callstack finishes.
[nelson] <lambda#1>::operator()() const server.cpp:234
[nelson] std::__invoke_impl<void, <lambda#1> &>(std::__invoke_other, <lambda#1> &) invoke.h:63
[nelson] std::__invoke_r<void, <lambda#1> &>(<lambda#1> &) invoke.h:113
[nelson] std::move_only_function<void ()>::_S_invoke<<lambda#1> >(std::_Mofunc_base *) mofunc_impl.h:219
[nelson] std::move_only_function<void ()>::operator() mofunc_impl.h:184
[nelson] uWS::Loop::wakeupCb Loop.h:50
[nelson] us_loop_run epoll_kqueue.c:147
[nelson] uWS::Loop::run Loop.h:216
[nelson] uWS::run Loop.h:233
[nelson] uWS::TemplatedApp<true>::run App.h:571
[nelson] ServerImpl::runApp server.cpp:215
[nelson] std::__invoke_impl<void, void (ServerImpl::*)(std::string), ServerImpl *, std::string> invoke.h:76
[nelson] std::__invoke<void (ServerImpl::*)(std::string), ServerImpl *, std::string> invoke.h:98
[nelson] std::thread::_Invoker<std::tuple<void (ServerImpl::*)(std::string), ServerImpl *, std::string> >::_M_invoke<0ul, 1ul, 2ul> std_thread.h:303
[nelson] std::thread::_Invoker<std::tuple<void (ServerImpl::*)(std::string), ServerImpl *, std::string> >::operator() std_thread.h:310
[nelson] std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (ServerImpl::*)(std::string), ServerImpl *, std::string> > >::_M_run std_thread.h:255
[libstdc++.so.6] std::execute_native_thread_routine 0x00007ffff64e51a4
[libasan.so.8] asan_thread_start 0x00007ffff785e29b
[libc.so.6] <unknown> 0x00007ffff62a57eb
[libc.so.6] <unknown> 0x00007ffff632918c
I setup the Websocket-functionality with
// WebSocket handlers
app.ws<WebsocketData>("/:0", {
.upgrade = std::bind_front(&ServerImpl::upgradeWebsocket, this),
.open = std::bind_front(&ServerImpl::onWebsocketOpen, this),
.message = [](auto *ws, std::string_view msg, uWS::OpCode op)
{
ws->send(msg, op); // Echo
},
.close = std::bind_front(&ServerImpl::onWebsocketClose, this)
});
With WebsocketData containing a std::string which is attempted to be destructed twice.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in WebSocket.h around WebSocket::end and follow the close-handler path into App.h around lines 263-439, especially the user-data destructor call. Reproduce the graceful shutdown with ASAN using the WebsocketData example, then verify that the close path destroys UserData only once and no longer reports the double-free.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 42/100