search

Cylix / cpp_redis

C++11 Lightweight Redis client: async, thread-safe, no dependency, pipelining, multi-platform - NO LONGER MAINTAINED - Please check https://github.com/cpp-redis/cpp_redis
MIT License
1.24k stars 551 forks source link

Unprotected member in subscriber during reconnect #168

Closed nkochakian closed 6 years ago

nkochakian commented 6 years ago

Before reconnecting, subscriber::connection_disconnection_handler acquires a lock (here) to prevent access to m_subscribed_channels. The corresponding lock for m_psubscribed_channels (m_psubscribed_channels_mutex) is never acquired. This can cause issues later when subscriber::re_subscribe is called, since it modifies m_psubscribed_channels.

A side effect of locking the channel map is that the connect callback is called in two different contexts:

  1. From subscriber::connect with no locks acquired
  2. From subscriber::connection_disconnection_handler via subscriber::reconnect when m_subscribed_channels_mutex is locked

For #2, calling subscriber::subscribe will cause a deadlock. (In the case of Visual C++ 2015, it might actually throw an exception, which is discarded by this catch all.)

Cylix commented 6 years ago

Hi @nkochakian,

Thanks for noticing that and reporting that, I will look into fixing that!

Best

Cylix commented 6 years ago

I pushed a fix on master.

Concerning your comment related to the side effect, I did not made any change as of now as it is anyway an existing problem in previous versions, in both redis subscriber and redis client.

I will record that as a possible improvement and work on that when I can.

Best