Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

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

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR37818
Status NEW
Importance P normal
Reported by Lewis G. Pringle Jr. (lewis@sophists.com)
Reported on 2018-06-15 13:29:37 -0700
Last modified on 2019-09-09 05:25:41 -0700
Version 6.0
Hardware PC All
CC hfinkel@anl.gov, llvm-bugs@lists.llvm.org, mclow.lists@gmail.com, prasadjoshi.linux@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
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<mutex> __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).
Quuxplusone 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.