Crow
Crow copied to clipboard
CrowCpp
Fix websocket crash from connection pointer dangling
On windows, with error 10053, a websocket instance sometimes deleted before post-ed/dispatch-ed asynchronous I/O events invocation, which makescompletion handler refer to dangled pointer 'this'.
We can detect whether 'this' pointer is deleted already or not, by checking captured weak_ptr is expired. Though this introduces a bit of overhead on each message posting/dispatching, it makes concurrent access to single websocket connection instance safe.
Hi! Thanks for the fix! Sorry for the long wait 😓
Lets ask someone who might have windows on hand @The-EDev if he can reproduce the original bug.
Else I'll check this out in a few days.
@dranikpg I haven't used windows in ages. Though this might be related to a problem I ran into in an early implementation of #269. I kept attempting to access Websocket connections after they've been deleted and couldn't figure out how or why.
- Resolved merge conflict with master branch
- Resolved compilation warning due to c++14 features
@dranikpg @kang-sw hey guys, is anything missing with this PR to move forward?
I would suggest explaining a thread-safe or at least a "recommended" way of using websocket::Connection outside of the asio handler flow. In a way it is related to #258 but taking into account the nature of websocket async communication might be even more urgent. In one way or another a server which needs to send a message to a connection has to capture a reference to the connection received much earlier in accept or previous messages. @dranikpg @kang-sw
Besides, I'm looking forward to #529 and refactor of websocket core parts. Crow is awesome :-) Thank you!
You're right, it's not documented so far. Websockets are increasingly popular in crow, but unfortunately pretty rough so far.
I wouldn't say its related to the async handlers issue because lifetime management is to be managed differently.
Thanks for using Crow in spite of its small flaws :)