drogonframework / drogonframework/drogon
AddressSanitizer: heap-use-after-free in TCPClient::removeConnection
- Dominant language
- C++
- Stars
- 14.3k
- Forks
- 1.4k
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 15
Description
**Describe the bug**
While running my Drogon application with the AndressSanitizer enabled it will occasionally terminate my application because it detects the use of an object after it's been freed. This happens when the TcpClient::removeConnection() method is called. In my application the Drogon application has a websocket connection to a client where it sends events. In some scenarios the client abruptly terminates (e.g. forcefully disconnects) while the application is trying to send these events. This is detected by the application and it will attempt to "stop" the websocket connection and tear it down. This works 99.8% of the time but in some "racey" situations, it appears Drogon/Trantor is attempting to call TcpClient::removeConnection() after the TcpClient has already been destroyed.
I looked at the code and the only place I see TcpClient::removeConnection() being called is from the connection callback which is initialized in TcpClient::newConnection, e.g.
`
conn->setCloseCallback(std::bind(&TcpClient::removeConnection, this, _1));
`
What has me concerned is that `conn` is a shared pointer and that callback function is being initialized by a raw `this` pointer to the TcpClient instance. It seems plausible there could be a scenario where the TcpClient instance has already been destroyed by the time the `conn` close callback is called. This seems to be the scenario I am seeing. I do see that TcpClient::~TcpClient destructor makes some effort to re-initialize the conn->setCloseCalllback() function but it's not clear if this is always being done within the scope/context of the destructor or scheduled to be done later in the event loop callback.
Is it possible to shed some light for the circumstances when TcpClient::removeConnection() should be called and why the raw `this` pointer being passed to the `conn->setCloseCallback()` method is not a potential problem? Again, I seem to see the issue more often when the Websocket client abruptly aborts the connection (e.g. no clean Websocket handshake to disconnect). How is the connection prevented from calling the TcpClient::removeConnection() method after it's been destroyed? Any insights or suggestions you could provide would be appreciated.
**To Reproduce**
More frequent when websocket client connect is abruptly disconnected by a client application.
**Expected behavior**
Don't call TcpClient::removeConnection() after TcpClient has been destroyed.
**Screenshots**
N/A
**Desktop (please complete the following information):**
- OS: Linux
Contributor guide
Assessment
This issue has not been assessed yet.