boostorg / thread

Boost.org thread module
http://boost.org/libs/thread
198 stars 162 forks source link

Seems starve writer @https://github.com/boostorg/thread/blob/boost-1.68.0/include/boost/thread/pthread/shared_mutex.hpp#L272 #265

Closed DengZuoheng closed 5 years ago

DengZuoheng commented 5 years ago
        void lock()
        {
#if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS
            boost::this_thread::disable_interruption do_not_disturb;
#endif
            boost::unique_lock<boost::mutex> lk(state_change);
            state.exclusive_waiting_blocked=true;
            exclusive_cond.wait(lk, boost::bind(&state_data::can_lock, boost::ref(state)));
            state.exclusive=true;
        }

In last reader unlock_shared, the state.exclusive_waiting_blocked will set to false, if there are writer and reader waiting at that time and reader wake up firstly and get a read-lock, then the writer need to wait again, but the state.exclusive_waiting_blocked would not set to true again for waiting writer.

Would it starve writer?

viboes commented 5 years ago

I'll need sometime to refresh my memory.

austin-beer commented 5 years ago

If I understand correctly, here is the scenario that @DengZuoheng has described.

R1 and R2 are both reader threads. W is a writer thread.

I think this situation is possible. However, I think it is extremely unlikely due to the fact that it requires R2 to call lock_shared() and be scheduled by the OS right between the time R1 calls unlock_shared() and the time W wakes up.

Boost.Thread contains another implementation of shared_mutex.hpp that was originally created by Howard Hinnant to replace the existing pthread/shared_mutex.hpp. This implementation can be used by building Boost and your application with BOOST_THREAD_V2_SHARED_MUTEX. I believe this implementation is simpler and more robust than the existing implementation. However, without a set of tests to prove that, it's probably too risky to replace the existing implementation that has been in use for years.

This V2 implementation would behave as follows in the above scenario.

viboes commented 5 years ago

I'm really sorry. I believed that BOOST_THREAD_V2_SHARED_MUTEX was defined when you define BOOST_THREAD_VERSION>=3 and it seems it is not the case.

Please, go ahead and try the V2 version by defining BOOST_THREAD_V2_SHARED_MUTEX and let us know if you are yet suffering from the same symptoms.

Thanks Austin for your help.

DengZuoheng commented 5 years ago

Thanks @austin-beer and @viboes for help, I will try shared_mutex v2, I will open new issue if I still suffering same symptoms.

AbdoSalem commented 2 years ago

Just to confirm that the situation described in https://github.com/boostorg/thread/issues/265#issuecomment-475903438 with the non v2 shared mutex, is reproducible. We have a situation where a unique lock starves in ~ 1 out of 5 times. The situation we have that increases the likelihood of the occurence of this issue is as follows:

This is just to reply to @austin-beer comment I think this situation is possible. However, I think it is extremely unlikely due to the fact that it requires R2 to call lock_shared() and be scheduled by the OS right between the time R1 calls unlock_shared() and the time W wakes up.

The above scenario was reproduced using 1.70. Previously, we did not experience such an issue but then we were on 1.58. We see that the above bhavior has changed on 1.67 and unfortunately there was no mention of it in the release notes.

JSoet commented 2 years ago

I'm not sure, but I think the example mentioned here: https://github.com/boostorg/thread/issues/361 Is the same issue, of writer starvation, and he included some sample code there to reproduce the problem. He also included a link to a commit which he said seems to have introduced the issue in boost 1.68