possible data race using packaged_task and threads

Question

I recently ran valgrind --tool=helgrind on my project and got a warning "possible data race", which I thought was concerning. However, even this simple test program leads to this message:

#include <iostream>
#include <thread>
#include <future>

int main()
{
   std::packaged_task<void()> task([]()
   {
      std::cout << "Hello\n"; // You can leave this out
   });
   auto future = task.get_future();
   std::thread(std::move(task)).detach();
   future.get();
}

Compiled with g++-4.9 -p -g -std=c++11 -pthread -O3 test.cpp, the output of valgrind --tool=helgrind ./a.out is:

==12808== Helgrind, a thread error detector
==12808== Copyright (C) 2007-2013, and GNU GPL'd, by OpenWorks LLP et al.
==12808== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==12808== Command: ./a.out
==12808== 
Hello
==12808== ---Thread-Announcement------------------------------------------
==12808== 
==12808== Thread #2 was created
==12808==    at 0x567B73E: clone (clone.S:74)
==12808==    by 0x536A1F9: do_clone.constprop.4 (createthread.c:75)
==12808==    by 0x536B7EB: create_thread (createthread.c:245)
==12808==    by 0x536B7EB: pthread_create@@GLIBC_2.2.5 (pthread_create.c:606)
==12808==    by 0x4C30E3D: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x4EF7F08: std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==12808==    by 0x403C3D: std::thread::thread<std::packaged_task<void ()>>(std::packaged_task<void ()>&&) (thread:138)
==12808==    by 0x401D57: main (test.cpp:13)
==12808== 
==12808== ----------------------------------------------------------------
==12808== 
==12808== Thread #2: pthread_cond_{signal,broadcast}: dubious: associated lock is not held by any thread
==12808==    at 0x4C2F295: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x4EF3CE8: std::condition_variable::notify_all() (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==12808==    by 0x402F18: _M_set_result (future:374)
==12808==    by 0x402F18: _M_run (future:1319)
==12808==    by 0x402F18: operator() (future:1453)
==12808==    by 0x402F18: _M_invoke<> (functional:1700)
==12808==    by 0x402F18: operator() (functional:1688)
==12808==    by 0x402F18: std::thread::_Impl<std::_Bind_simple<std::packaged_task<void ()> ()> >::_M_run() (thread:115)
==12808==    by 0x4EF7DCF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==12808==    by 0x4C30FD6: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x536B0A4: start_thread (pthread_create.c:309)
==12808==    by 0x567B77C: clone (clone.S:111)
==12808== 
==12808== ---Thread-Announcement------------------------------------------
==12808== 
==12808== Thread #1 is the program's root thread
==12808== 
==12808== ----------------------------------------------------------------
==12808== 
==12808== Possible data race during read of size 8 at 0x5C4FAA0 by thread #1
==12808== Locks held: none
==12808==    at 0x4026A9: ~unique_ptr (unique_ptr.h:235)
==12808==    by 0x4026A9: ~_Task_state_base (future:1281)
==12808==    by 0x4026A9: ~_Task_state (future:1302)
==12808==    by 0x4026A9: destroy<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()> > (new_allocator.h:124)
==12808==    by 0x4026A9: _S_destroy<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()> > (alloc_traits.h:282)
==12808==    by 0x4026A9: destroy<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()> > (alloc_traits.h:411)
==12808==    by 0x4026A9: std::_Sp_counted_ptr_inplace<std::__future_base::_Task_state<main::{lambda()#1}, std::allocator<int>, void ()>, main::{lambda()#1}, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:524)
==12808==    by 0x401ED4: _M_release (shared_ptr_base.h:149)
==12808==    by 0x401ED4: ~__shared_count (shared_ptr_base.h:666)
==12808==    by 0x401ED4: ~__shared_ptr (shared_ptr_base.h:914)
==12808==    by 0x401ED4: reset (shared_ptr_base.h:1015)
==12808==    by 0x401ED4: ~_Reset (future:657)
==12808==    by 0x401ED4: get (future:786)
==12808==    by 0x401ED4: main (test.cpp:14)
==12808== 
==12808== This conflicts with a previous write of size 8 by thread #2
==12808== Locks held: none
==12808==    at 0x403407: release (unique_ptr.h:328)
==12808==    by 0x403407: unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter, void> (unique_ptr.h:221)
==12808==    by 0x403407: ~packaged_task (future:1409)
==12808==    by 0x403407: ~_Head_base (tuple:128)
==12808==    by 0x403407: ~_Tuple_impl (tuple:229)
==12808==    by 0x403407: ~tuple (tuple:388)
==12808==    by 0x403407: ~_Bind_simple (functional:1663)
==12808==    by 0x403407: std::thread::_Impl<std::_Bind_simple<std::packaged_task<void ()> ()> >::~_Impl() (thread:107)
==12808==    by 0x4EF7E1A: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==12808==    by 0x4C30FD6: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x536B0A4: start_thread (pthread_create.c:309)
==12808==    by 0x567B77C: clone (clone.S:111)
==12808==  Address 0x5c4faa0 is 128 bytes inside a block of size 144 alloc'd
==12808==    at 0x4C2C520: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x401C8C: allocate (new_allocator.h:104)
==12808==    by 0x401C8C: allocate (alloc_traits.h:357)
==12808==    by 0x401C8C: __shared_count<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()>, std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr_base.h:616)
==12808==    by 0x401C8C: __shared_ptr<std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr_base.h:1090)
==12808==    by 0x401C8C: shared_ptr<std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr.h:316)
==12808==    by 0x401C8C: allocate_shared<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()>, std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr.h:588)
==12808==    by 0x401C8C: __create_task_state<void(), main()::<lambda()>, std::allocator<int> > (future:1351)
==12808==    by 0x401C8C: packaged_task<main()::<lambda()>, std::allocator<int>, void> (future:1403)
==12808==    by 0x401C8C: packaged_task<main()::<lambda()>, void> (future:1393)
==12808==    by 0x401C8C: main (test.cpp:11)
==12808==  Block was alloc'd by thread #1
==12808== 
==12808== ----------------------------------------------------------------
==12808== 
==12808== Possible data race during read of size 1 at 0x5C4FA6C by thread #1
==12808== Locks held: none
==12808==    at 0x4C2ED90: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x4C2F57F: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x4026D7: ~_State_baseV2 (future:316)
==12808==    by 0x4026D7: ~_Task_state_base (future:1281)
==12808==    by 0x4026D7: ~_Task_state (future:1302)
==12808==    by 0x4026D7: destroy<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()> > (new_allocator.h:124)
==12808==    by 0x4026D7: _S_destroy<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()> > (alloc_traits.h:282)
==12808==    by 0x4026D7: destroy<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()> > (alloc_traits.h:411)
==12808==    by 0x4026D7: std::_Sp_counted_ptr_inplace<std::__future_base::_Task_state<main::{lambda()#1}, std::allocator<int>, void ()>, main::{lambda()#1}, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:524)
==12808==    by 0x401ED4: _M_release (shared_ptr_base.h:149)
==12808==    by 0x401ED4: ~__shared_count (shared_ptr_base.h:666)
==12808==    by 0x401ED4: ~__shared_ptr (shared_ptr_base.h:914)
==12808==    by 0x401ED4: reset (shared_ptr_base.h:1015)
==12808==    by 0x401ED4: ~_Reset (future:657)
==12808==    by 0x401ED4: get (future:786)
==12808==    by 0x401ED4: main (test.cpp:14)
==12808== 
==12808== This conflicts with a previous write of size 4 by thread #2
==12808== Locks held: none
==12808==    at 0x536F9BF: pthread_cond_broadcast@@GLIBC_2.3.2 (pthread_cond_broadcast.S:59)
==12808==    by 0x402F18: _M_set_result (future:374)
==12808==    by 0x402F18: _M_run (future:1319)
==12808==    by 0x402F18: operator() (future:1453)
==12808==    by 0x402F18: _M_invoke<> (functional:1700)
==12808==    by 0x402F18: operator() (functional:1688)
==12808==    by 0x402F18: std::thread::_Impl<std::_Bind_simple<std::packaged_task<void ()> ()> >::_M_run() (thread:115)
==12808==  Address 0x5c4fa6c is 76 bytes inside a block of size 144 alloc'd
==12808==    at 0x4C2C520: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==12808==    by 0x401C8C: allocate (new_allocator.h:104)
==12808==    by 0x401C8C: allocate (alloc_traits.h:357)
==12808==    by 0x401C8C: __shared_count<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()>, std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr_base.h:616)
==12808==    by 0x401C8C: __shared_ptr<std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr_base.h:1090)
==12808==    by 0x401C8C: shared_ptr<std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr.h:316)
==12808==    by 0x401C8C: allocate_shared<std::__future_base::_Task_state<main()::<lambda()>, std::allocator<int>, void()>, std::allocator<int>, main()::<lambda()>, const std::allocator<int>&> (shared_ptr.h:588)
==12808==    by 0x401C8C: __create_task_state<void(), main()::<lambda()>, std::allocator<int> > (future:1351)
==12808==    by 0x401C8C: packaged_task<main()::<lambda()>, std::allocator<int>, void> (future:1403)
==12808==    by 0x401C8C: packaged_task<main()::<lambda()>, void> (future:1393)
==12808==    by 0x401C8C: main (test.cpp:11)
==12808==  Block was alloc'd by thread #1
==12808== 
==12808== 
==12808== For counts of detected and suppressed errors, rerun with: -v
==12808== Use --history-level=approx or =none to gain increased speed, at
==12808== the cost of reduced accuracy of conflicting-access information
==12808== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 23 from 23)

My question: Am I safe to ignore this? If so, how to suppress these warnings?

Answer 1

I also ran into this warning, and checked the source code of libstdc++ for GCC 4.8 (/usr/include/c++/4.8/future) on Kubuntu 14.04. I found no bug. The future's state contains, among other things, a result pointer, a mutex and a condition variable. future<T>::wait() locks the mutex and checks whether the result pointer is null. If it is, it waits on the condition variable (which will unlock the mutex as well in one atomic operation). On promise<T>::set_value() (or set_exception(), I guess), the code locks the mutex, sets the result pointer, unlocks the mutex, and invokes notify_all() on the condition variable. Hence Helgrind warns because the mutex is not locked when notify_all() is called. However, the code is still safe because the mutex is held for setting the pointer, which is the only way to change the outcome of the check in future<T>::wait(). Doing the notify outside of the life time of the lock_guard may be an optimization (holding the mutex as briefly as possible). It does unfortunately lead to this warning. libstdc++ maintainers will probably be reluctant about accepting a patch, because a) a patch would reduce performance, if slightly, and b) don't fix it if it ain't broken. My recommendation therefore is to make a suppression for Helgrind.

Please note that I simplified some interactions for clarity and brevity, in particular future and promise delegate the actual work to hidden implementation classes, set_value uses an additional once_flag to synchronize concurrent calls, etc.

See also this thread, my answer to it and the precise comment to my answer by Cubbi (something I had overlooked): Why do both the notify and wait function of a std::condition_variable need a locked mutex