llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.57k forks source link

condition_variable::wait_for () should always unlock/lock #37166

Open llvmbot opened 6 years ago

llvmbot commented 6 years ago
Bugzilla Link 37818
Version 6.0
OS All
Reporter LLVM Bugzilla Contributor
CC @hfinkel,@mclow

Extended Description

Note - similar to 21395, except reasoning is very different.

http://en.cppreference.com/w/cpp/thread/condition_variable/wait_for says:

... Atomically releases lock, blocks the current executing thread, and adds it to the list of threads waiting on *this

For the most part this is true with the libc++ implementation, but not when if (d <= d.zero()) return cv_status::timeout;

for comparison, see libstdc++ does (https://gcc.gnu.org/onlinedocs/gcc-4.6.2/libstdc++/api/a00818_source.html) template<typename _Lock, typename _Clock, typename _Duration> 00217 cv_status 00218 wait_until(_Lock& lock, 00219 const chrono::time_point<_Clock, _Duration>& __atime) 00220 { 00221 unique_lock my_lock(_M_mutex); 00222 lock.unlock(); 00223 cv_status __status = _M_cond.wait_until(my_lock, atime); 00224 lock.lock(); 00225 return __status; 00226 }

unconditionally unlock/lock (even if we are going to get a timeout).

This is IMPORTANT, because if you wait_for (0) a bunch of times (stupid but not illegal). and in another thread try to get the mutex for the variable, before setting it, you may infinite loop (because the 'signal'ing thread never gets the lock).

mclow commented 6 years ago

I appreciate the explanation, but for future reference, please don't post libstdc++ code here. For licensing reasons, we can't make use of it. Thanks.