search

heroiclabs / nakama-cpp

Generic C/C++ client for Nakama server.
https://heroiclabs.com/docs/cpp-client-guide
Apache License 2.0
68 stars 25 forks source link

Perform disconnect() after wslay_event_read when remotely disconnected #73

Closed lugehorsam closed 1 year ago

lugehorsam commented 1 year ago

If the socket was remotely disconnected, the wslay context would be reset from within the wslay_event_read call because we immediately call disconnect() upon receiving a close frame https://github.com/heroiclabs/nakama-cpp/compare/luke/tick?expand=1#diff-0a230c967b82d8b2272c6104259685dc6db24050c0a9473b59bd852f9ca65ed5L290

This would cause a bad access.

I looked at a few different options but ultimately landed on adding a new state to the state machine so that the client can perform it's final disconnection logic in the next tick(). But there are probably a few different valid ways of solving this.

redbaron commented 1 year ago

Can you help me understand how does it fix bad access?

Before sequence was like this:

tick()
  \ -> wslay_event_recv() -> ... -> on_msg_recv_callback() -> disconnect(true) -> fireOnDisconnected()

With this change:

tick()
  \ -> wslay_event_recv() -> ... -> on_msg_recv_callback() // sets State::RemoteDisconnect
  \ -> disconnect(true) -> fireOnDisconnected()

So even if nesting is different, total order of function calls of interest is the same.

EDIT:

Oh, I see, it is 

on_msg_recv_callback() -> disconnect() -> _ctx.reset(nullptr);

which is a problem, because we are destroying wslay context from within wslay callback. Fix make sense.