[Bug libstdc++/60966] New: std::call_once sometime hangs

[Bug libstdc++/60966] New: std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

            Bug ID: 60966
           Summary: std::call_once sometime hangs
           Product: gcc
           Version: 4.8.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: thomas.sanchz at gmail dot com

Hi,
I'm really sorry for the vague bug report and not being able to give a small
piece of code to reproduce the bug.

Here is what I'm experiencing:

I developed a small library which performs http request, the library is built
around curl.
I'm using std::promise/future in order to push a task to do in the thread were
all the operation on the handle is done.

for example, here is an example of some of the piece of the code I have:

    {
        std::promise<void> promise;
        auto future = promise.get_future();
        io.dispatch([this, &promise]
                    {
                        checkHandles();
                        promise.set_value();
                    });
        future.get();
    }

Sometimes, it can hangs on the set_value() with this kind of stack trace:

(gdb) bt
#0  pthread_cond_wait@@GLIBC_2.3.2 () at
../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x00007ffff74f766c in
std::condition_variable::wait(std::unique_lock<std::mutex>&) () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
#2  0x0000000000647555 in
std::condition_variable::wait<std::__future_base::_State_base::wait()::{lambda()#1}>(std::unique_lock<std::mutex>&,
std::__future_base::_State_base::wait()::{lambda()#1}) (this=0xa28990,
__lock=..., __p=...) at /usr/include/c++/4.8/condition_variable:93
#3  0x0000000000643d1c in std::__future_base::_State_base::wait (this=0xa28958)
at /usr/include/c++/4.8/future:327
#4  0x00000000006b29bf in std::__basic_future<void>::_M_get_result
(this=0x7fffffffa150) at /usr/include/c++/4.8/future:595
#5  0x00000000006b1626 in std::future<void>::get (this=0x7fffffffa150) at
/usr/include/c++/4.8/future:760
#6  0x00000000006acb21 in HTTPP::HTTP::client::detail::Manager::~Manager
(this=0x9efe90, __in_chrg=<optimized out>) at
/home/daedric/perso/httpp/src/httpp/http/client/Manager.cpp:113
#7  0x000000000069d92a in
std::default_delete<HTTPP::HTTP::client::detail::Manager>::operator()
(this=0x9f1168, __ptr=0x9efe90) at /usr/include/c++/4.8/bits/unique_ptr.h:67
#8  0x000000000069d53c in std::unique_ptr<HTTPP::HTTP::client::detail::Manager,
std::default_delete<HTTPP::HTTP::client::detail::Manager> >::reset
(this=0x9f1168, __p=0x9efe90) at /usr/include/c++/4.8/bits/unique_ptr.h:262
#9  0x000000000069b17f in HTTPP::HttpClient::~HttpClient (this=0x9f10c8,
__in_chrg=<optimized out>) at
/home/daedric/perso/httpp/src/httpp/HttpClient.cpp:48
#10 0x000000000065724e in etcd::Etcd::Connection::~Connection (this=0x9f10c0,
__in_chrg=<optimized out>) at /home/daedric/work/broker/broker/etcd/Etcd.cpp:28
#11 0x000000000065727e in
std::default_delete<etcd::Etcd::Connection>::operator() (this=0xa27780,
__ptr=0x9f10c0) at /usr/include/c++/4.8/bits/unique_ptr.h:67
#12 0x0000000000655f1b in std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> >::~unique_ptr (this=0xa27780,
__in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/unique_ptr.h:184
#13 0x000000000065483e in std::_Destroy<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> > > (__pointer=0xa27780) at
/usr/include/c++/4.8/bits/stl_construct.h:93
#14 0x000000000065295c in
std::_Destroy_aux<false>::__destroy<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> >*> (__first=0xa27780,
__last=0xa27788) at /usr/include/c++/4.8/bits/stl_construct.h:103
#15 0x000000000064fefb in std::_Destroy<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> >*> (__first=0xa27780,
__last=0xa27788) at /usr/include/c++/4.8/bits/stl_construct.h:126
#16 0x000000000064c065 in std::_Destroy<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> >*,
std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> > > (__first=0xa27780,
__last=0xa27788) at /usr/include/c++/4.8/bits/stl_construct.h:151
#17 0x000000000064d4c8 in std::vector<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> >,
std::allocator<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> > > >::_M_erase_at_end
(this=0x7fffffffaf00, __pos=0xa27780)
    at /usr/include/c++/4.8/bits/stl_vector.h:1352
#18 0x00000000006489c0 in std::vector<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> >,
std::allocator<std::unique_ptr<etcd::Etcd::Connection,
std::default_delete<etcd::Etcd::Connection> > > >::clear (this=0x7fffffffaf00)
at /usr/include/c++/4.8/bits/stl_vector.h:1126
#19 0x000000000063ccb0 in etcd::Etcd::~Etcd (this=0x7fffffffaea0,
__in_chrg=<optimized out>) at
/home/daedric/work/broker/broker/etcd/Etcd.cpp:264
#20 0x000000000063118c in basic::test_method (this=0x7fffffffd68f) at
/home/daedric/work/broker/tests/etcd/tree.cpp:70
#21 0x00000000006300cf in basic_invoker () at
/home/daedric/work/broker/tests/etcd/tree.cpp:12
#22 0x000000000063bd64 in
boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void
(*)()> (this=0x7fffffffd6ef, f=@0x9e8048: 0x6300bb <basic_invoker()>) at
/usr/include/boost/test/utils/callback.hpp:56
#23 0x000000000063bbb4 in
boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused,
void (*)()>::invoke (this=0x9e8040) at
/usr/include/boost/test/utils/callback.hpp:89
#24 0x00000000006ded01 in boost::unit_test::ut_detail::callback0_impl_t<int,
boost::unit_test::(anonymous
namespace)::zero_return_wrapper_t<boost::unit_test::callback0<boost::unit_test::ut_detail::unused>
> >::invoke() ()
#25 0x00000000006feeb6 in
boost::execution_monitor::catch_signals(boost::unit_test::callback0<int>
const&) ()
#26 0x00000000006ff6c3 in
boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) ()
#27 0x00000000006deddb in
boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case
const&) ()
#28 0x00000000006cd8a4 in
boost::unit_test::framework_impl::visit(boost::unit_test::test_case const&) ()
#29 0x00000000006f62dc in
boost::unit_test::traverse_test_tree(boost::unit_test::test_suite const&,
boost::unit_test::test_tree_visitor&) ()
#30 0x00000000006c902b in boost::unit_test::framework::run(unsigned long, bool)
()
#31 0x00000000006dca94 in
boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int,
char**), int, char**) ()
#32 0x00007ffff6e8eec5 in __libc_start_main (main=0x62caa0 <main>, argc=1,
argv=0x7fffffffe4e8, init=<optimized out>, fini=<optimized out>,
rtld_fini=<optimized out>, stack_end=0x7fffffffe4d8) at libc-start.c:287
#33 0x000000000062ff7f in _start ()
(gdb) 
(gdb) thread 2
[Switching to thread 2 (Thread 0x7ffff2011700 (LWP 5256))]
#0  pthread_once () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_once.S:94
94    ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_once.S: No such file or
directory.
(gdb) bt
#0  pthread_once () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_once.S:94
#1  0x000000000069e5c3 in __gthread_once (__once=0xa289c4, __func=0x62ab50
<__once_proxy@plt>) at
/usr/include/x86_64-linux-gnu/c++/4.8/bits/gthr-default.h:699
#2  0x00000000006a2cd9 in std::call_once<void
(std::__future_base::_State_base::*)(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>&, bool&),
std::__future_base::_State_base* const,
std::reference_wrapper<std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()> >, std::reference_wrapper<bool>
>(std::once_flag&, void
(std::__future_base::_State_base::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>&, bool&),
std::__future_base::_State_base* const&&,
std::reference_wrapper<std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()> >&&,
std::reference_wrapper<bool>&&) (__once=..., __f=<unknown type in
/home/daedric/work/broker/build/tests/etcd/Test_Etcd_tree, CU 0x2582b2, DIE
0x296a80>)
    at /usr/include/c++/4.8/mutex:786
#3  0x00000000006a1689 in
std::__future_base::_State_base::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>, bool) (this=0xa28958,
__res=..., __ignore_failure=false) at /usr/include/c++/4.8/future:358
#4  0x00000000006b19df in std::promise<void>::set_value (this=0x7fffffffa1a0)
at /usr/include/c++/4.8/future:1197
#5  0x00000000006ac879 in
HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#2}::operator()()
const () at /home/daedric/perso/httpp/src/httpp/http/client/Manager.cpp:111
#6  0x00000000006b0a05 in
boost::asio::asio_handler_invoke<HTTPP::HTTP::client::detail::Manager::~Manager()::__lambda5>(HTTPP::HTTP::client::detail::Manager::__lambda5,
...) (function=...) at /usr/include/boost/asio/handler_invoke_hook.hpp:64
#7  0x00000000006b0448 in
boost_asio_handler_invoke_helpers::invoke<HTTPP::HTTP::client::detail::Manager::~Manager()::__lambda5,
HTTPP::HTTP::client::detail::Manager::~Manager()::__lambda5>(HTTPP::HTTP::client::detail::Manager::__lambda5
&, HTTPP::HTTP::client::detail::Manager::__lambda5 &) (function=..., 
    context=...) at
/usr/include/boost/asio/detail/handler_invoke_helpers.hpp:37
#8  0x00000000006b0b3c in
boost::asio::detail::completion_handler<HTTPP::HTTP::client::detail::Manager::~Manager()::__lambda5>::do_complete(boost::asio::detail::io_service_impl
*, boost::asio::detail::operation *, const boost::system::error_code &,
std::size_t) (owner=0x9f0830, base=0xa2bd20)
    at /usr/include/boost/asio/detail/completion_handler.hpp:68
#9  0x00000000006a1970 in
boost::asio::detail::task_io_service_operation::complete (this=0xa2bd20,
owner=..., ec=..., bytes_transferred=0) at
/usr/include/boost/asio/detail/task_io_service_operation.hpp:37
#10 0x00000000006c21b3 in boost::asio::detail::task_io_service::do_run_one
(this=0x9f0830, lock=..., this_thread=..., ec=...) at
/usr/include/boost/asio/detail/impl/task_io_service.ipp:384
#11 0x00000000006c1eb7 in boost::asio::detail::task_io_service::run
(this=0x9f0830, ec=...) at
/usr/include/boost/asio/detail/impl/task_io_service.ipp:153
#12 0x00000000006c246d in boost::asio::io_service::run (this=0x9efa48) at
/usr/include/boost/asio/impl/io_service.ipp:59
#13 0x00000000006bf00b in HTTPP::UTILS::ThreadPool::run(std::function<void ()>)
(this=0x9f10c8, fct=...) at
/home/daedric/perso/httpp/src/httpp/utils/ThreadPool.cpp:114
#14 0x00000000006c0d0e in std::_Mem_fn<void
(HTTPP::UTILS::ThreadPool::*)(std::function<void()>)>::operator()<HTTPP::UTILS::ThreadPool::start(HTTPP::UTILS::ThreadPool::ThreadInit)::__lambda0&,
void>(HTTPP::UTILS::ThreadPool *) const (this=0xa275c0, __object=0x9f10c8) at
/usr/include/c++/4.8/functional:601
#15 0x00000000006c0bd1 in std::_Bind<std::_Mem_fn<void
(HTTPP::UTILS::ThreadPool::*)(std::function<void()>)>(HTTPP::UTILS::ThreadPool*,
HTTPP::UTILS::ThreadPool::start(HTTPP::UTILS::ThreadPool::ThreadInit)::__lambda0)>::__call<void,
0ul, 1ul>(<unknown type in
/home/daedric/work/broker/build/tests/etcd/Test_Etcd_tree, CU 0x322b8a, DIE
0x3501ae>, std::_Index_tuple<0ul, 1ul>) (this=0xa275c0, __args=<unknown type in
/home/daedric/work/broker/build/tests/etcd/Test_Etcd_tree, CU 0x322b8a, DIE
0x3501ae>) at /usr/include/c++/4.8/functional:1296
#16 0x00000000006c0aec in std::_Bind<std::_Mem_fn<void
(HTTPP::UTILS::ThreadPool::*)(std::function<void()>)>(HTTPP::UTILS::ThreadPool*,
HTTPP::UTILS::ThreadPool::start(HTTPP::UTILS::ThreadPool::ThreadInit)::__lambda0)>::operator()<,
void>(void) (this=0xa275c0) at /usr/include/c++/4.8/functional:1355
#17 0x00000000006c0a7e in std::_Bind_simple<std::_Bind<std::_Mem_fn<void
(HTTPP::UTILS::ThreadPool::*)(std::function<void()>)>(HTTPP::UTILS::ThreadPool*,
HTTPP::UTILS::ThreadPool::start(HTTPP::UTILS::ThreadPool::ThreadInit)::__lambda0)>()>::_M_invoke<>(std::_Index_tuple<>)
(this=0xa275c0)
    at /usr/include/c++/4.8/functional:1732
#18 0x00000000006c09d5 in std::_Bind_simple<std::_Bind<std::_Mem_fn<void
(HTTPP::UTILS::ThreadPool::*)(std::function<void()>)>(HTTPP::UTILS::ThreadPool*,
HTTPP::UTILS::ThreadPool::start(HTTPP::UTILS::ThreadPool::ThreadInit)::__lambda0)>()>::operator()(void)
(this=0xa275c0) at /usr/include/c++/4.8/functional:1720
#19 0x00000000006c096e in
std::thread::_Impl<std::_Bind_simple<std::_Bind<std::_Mem_fn<void
(HTTPP::UTILS::ThreadPool::*)(std::function<void()>)>(HTTPP::UTILS::ThreadPool*,
HTTPP::UTILS::ThreadPool::start(HTTPP::UTILS::ThreadPool::ThreadInit)::__lambda0)>()>
>::_M_run(void) (this=0xa275a8)
    at /usr/include/c++/4.8/thread:115
#20 0x00007ffff74fabf0 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#21 0x00007ffff7bc4182 in start_thread (arg=0x7ffff2011700) at
pthread_create.c:312
#22 0x00007ffff6f6830d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:111


Valgrind showed this on another run:

==3079== Thread 2:
==3079== Invalid read of size 4
==3079==    at 0x4E44A91: pthread_once (pthread_once.S:111)
==3079==    by 0x69E5C2: __gthread_once(int*, void (*)()) (gthr-default.h:699)
==3079==    by 0x6A2CD8: void std::call_once<void
(std::__future_base::_State_base::*)(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>&, bool&),
std::__future_base::_State_base* const,
std::reference_wrapper<std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()> >, std::reference_wrapper<bool>
>(std::once_flag&, void
(std::__future_base::_State_base::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>&, bool&),
std::__future_base::_State_base* const&&,
std::reference_wrapper<std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()> >&&,
std::reference_wrapper<bool>&&) (mutex:786)
==3079==    by 0x6A1688:
std::__future_base::_State_base::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>, bool) (future:358)
==3079==    by 0x6B19DE: std::promise<void>::set_value() (future:1197)
==3079==    by 0x6AC7D6:
HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#1}::operator()()
const (Manager.cpp:90)
==3079==    by 0x6B0809: void
boost::asio::asio_handler_invoke<HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#1}>(HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#1},
...) (handler_invoke_hook.hpp:64)
==3079==    by 0x6B0352: void
boost_asio_handler_invoke_helpers::invoke<HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#1},
{lambda()#1}>(HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#1}&,
{lambda()#1}&) (handler_invoke_helpers.hpp:37)
==3079==    by 0x6B0947:
boost::asio::detail::completion_handler<HTTPP::HTTP::client::detail::Manager::~Manager()::{lambda()#1}>::do_complete(boost::asio::detail::task_io_service*,
boost::asio::detail::task_io_service_operation*, boost::system::error_code
const&, unsigned long) (completion_handler.hpp:68)
==3079==    by 0x6A196F:
boost::asio::detail::task_io_service_operation::complete(boost::asio::detail::task_io_service&,
boost::system::error_code const&, unsigned long)
(task_io_service_operation.hpp:37)
==3079==    by 0x6C21B2:
boost::asio::detail::task_io_service::do_run_one(boost::asio::detail::scoped_lock<boost::asio::detail::posix_mutex>&,
boost::asio::detail::task_io_service_thread_info&, boost::system::error_code
const&) (task_io_service.ipp:384)
==3079==    by 0x6C1EB6:
boost::asio::detail::task_io_service::run(boost::system::error_code&)
(task_io_service.ipp:153)
==3079==  Address 0xdf926d4 is 132 bytes inside a block of size 136 free'd
==3079==    at 0x4C2C2BC: operator delete(void*) (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3079==    by 0x6AA993:
__gnu_cxx::new_allocator<std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>
>::deallocate(std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>*,
unsigned long) (new_allocator.h:110)
==3079==    by 0x6AA570:
std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2> >
>::deallocate(std::allocator<std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>
>&, std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>*,
unsigned long) (alloc_traits.h:377)
==3079==    by 0x6AB95D:
std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>,
(__gnu_cxx::_Lock_policy)2>::_M_destroy() (shared_ptr_base.h:417)
==3079==    by 0x64977D:
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release()
(shared_ptr_base.h:161)
==3079==    by 0x645AA8:
std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
(shared_ptr_base.h:546)
==3079==    by 0x643E09: std::__shared_ptr<std::__future_base::_State_base,
(__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:781)
==3079==    by 0x69CD89:
std::shared_ptr<std::__future_base::_State_base>::~shared_ptr()
(shared_ptr.h:93)
==3079==    by 0x6B1809: std::promise<void>::~promise() (future:1136)
==3079==    by 0x6AC9C9: HTTPP::HTTP::client::detail::Manager::~Manager()
(Manager.cpp:93)
==3079==    by 0x69D929:
std::default_delete<HTTPP::HTTP::client::detail::Manager>::operator()(HTTPP::HTTP::client::detail::Manager*)
const (unique_ptr.h:67)
==3079==    by 0x69D53B: std::unique_ptr<HTTPP::HTTP::client::detail::Manager,
std::default_delete<HTTPP::HTTP::client::detail::Manager>
>::reset(HTTPP::HTTP::client::detail::Manager*) (unique_ptr.h:262)


For some reason the bug seems to occurs *ONLY* when I use promise/future in the
destructor or in function called from the destructor. Could it be related to
some optimization performed by either the compiler or the linker ?
How can I help to debug such an issue ?

If it can be of any help the code I've problem with is the destructor of this
class:
https://github.com/daedric/httpp/blob/master/src/httpp/http/client/Manager.cpp

I'm using GCC 4.8.2 on an ubuntu 14.04 with the default library.

Thanks,

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(It would be easier to make sense of this if the line numbers in your gdb and
valgrind output matched the code on github.)

Blocking in pthread_once could be a symptom of using an invalid pthread_once_t
that has already been destroyed, and the valgrind output indeed shows that the
set_value() call is being made on a shared state that has been deleted already,
when the promise that owned it went out of scope. It looks like that shouldn't
have happened though.

Is it possible the task is getting run twice by the thread pool, so on the
second run the reference to the promise is dangling?

When the process hangs could you use gdb to print &promise in both threads,
where it is waiting on future::get and promise::set_value? The values should be
the same, since the closure running in the thread pool should have a reference
to the local object in the waiting thread.

Replacing the lambda in the destructor with a call to a member function would
help to rule out a code generation problem due to the lambda, which is a remote
possibility.

Can the problem be reproduced by one of the unit tests in the httpp repo?


N.B. the std::move in Manager::cancelConnection is redundant, the return value
from cancel_connection is already an rvalue.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> > N.B. the std::move in Manager::cancelConnection is redundant, the return
> > value from cancel_connection is already an rvalue.
> 
> Yes, I know, the problem is I have some hard time to avoid doing this as it
> is more for me and what I expect the compiler to do or the semantics I want
> the code to have :)

OK, but it makes the code worse. Here the compiler will elide the move
constructor (aka return value optimisation):

  auto future = cancel_connection(c);

Whereas here the compiler cannot do that and must call a move constructor:

  auto future = std::move(cancel_connection(c));

[Bug libstdc++/60966] std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #4 from Thomas Sanchez <thomas.sanchz at gmail dot com> ---
(In reply to Jonathan Wakely from comment #3)
> > > N.B. the std::move in Manager::cancelConnection is redundant, the return
> > > value from cancel_connection is already an rvalue.
> > 
> > Yes, I know, the problem is I have some hard time to avoid doing this as it
> > is more for me and what I expect the compiler to do or the semantics I want
> > the code to have :)
> 
> OK, but it makes the code worse. Here the compiler will elide the move
> constructor (aka return value optimisation):
> 
>   auto future = cancel_connection(c);
> 
> Whereas here the compiler cannot do that and must call a move constructor:
> 
>   auto future = std::move(cancel_connection(c));

I did not meant to say I should continue, far from here :) Thank for the
notice!

Anyway, I pushed the actual code in this branch:
https://github.com/daedric/httpp/tree/bug-promise-test

I was able to do a small test that can *sometimes* reproduce the bug:
https://github.com/daedric/httpp/blob/bug-promise-test/tests/client/promise.cpp


Since I was able to trigger the problem, here are what you asked for:
>From the get side, (from here
https://github.com/daedric/httpp/blob/bug-promise-test/src/httpp/http/client/Manager.cpp#L113)

(gdb) print future
$1 = {
  <std::__basic_future<void>> = {
    <std::__future_base> = {<No data fields>}, 
    members of std::__basic_future<void>: 
    _M_state = std::shared_ptr (count 2, weak 0) 0x805468
  }, <No data fields>}
(gdb) print promise
$2 = {
  _M_future = std::shared_ptr (count 2, weak 0) 0x805468, 
  _M_storage = std::unique_ptr<std::__future_base::_Result<void>> containing
0x805780
}
(gdb) print &promise
$3 = (std::promise<void> *) 0x7fffffffce90


>From the thread calling the pthread_once:

#4  0x0000000000500d67 in std::promise<void>::set_value (this=0x7fffffffce90)
at /usr/include/c++/4.8/future:1197
1197        _M_future->_M_set_result(std::move(__setter));
(gdb) print *this
$4 = {
  _M_future = std::shared_ptr (count 2, weak 0) 0x805468, 
  _M_storage = std::unique_ptr<std::__future_base::_Result<void>> containing
0x805780
}

This is seems to be correct.
This has been produced with commit: 850ebba9a72d102b54de6912e820889618a4f30d

I'm testing now with the lambdas replaced :)





http://reproducible.io/

[Bug libstdc++/60966] std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #5 from Thomas Sanchez <thomas.sanchz at gmail dot com> ---
With my previous mail, the link I added was because I wanted to ask that if you
were not able to reproduce the problem I could use CARE to recreate the
environment I'm running my test.

With lambda replaced (commit fd8b4a8ce7847dc561c937e2a3c80db0d735835c) the
seems to be triggered a bit less often however, I do have a segfault in
malloc_consolidate, I'll come back onto this later.

Manager.cpp:121
(gdb) print future
$1 = {
  <std::__basic_future<void>> = {
    <std::__future_base> = {<No data fields>}, 
    members of std::__basic_future<void>: 
    _M_state = std::shared_ptr (count 2, weak 0) 0x812468
  }, <No data fields>}
(gdb) print promise
$2 = {
  _M_future = std::shared_ptr (count 2, weak 0) 0x812468, 
  _M_storage = std::unique_ptr<std::__future_base::_Result<void>> containing
0x812780
}

Manager.cpp:87
(gdb) print promise
$3 = (std::promise<void> &) @0x7fffffffce40: {
  _M_future = std::shared_ptr (count 2, weak 0) 0x812468, 
  _M_storage = std::unique_ptr<std::__future_base::_Result<void>> containing
0x812780
}

This also seems ok.

About the segfault on the malloc, I think the problems are related, I guess it
is possible that the pthread_once does not always hangs and who knows what mess
it could cause in the memory.

In case you interested in the segfault, here is the trace:
Program received signal SIGSEGV, Segmentation fault.
malloc_consolidate (av=av@entry=0x7ffff722c760 <main_arena>) at malloc.c:4157
4157    malloc.c: No such file or directory.
(gdb) bt
#0  malloc_consolidate (av=av@entry=0x7ffff722c760 <main_arena>) at
malloc.c:4157
#1  0x00007ffff6eedc38 in _int_malloc (av=0x7ffff722c760 <main_arena>,
bytes=7288) at malloc.c:3423
#2  0x00007ffff6ef05f0 in __GI___libc_malloc (bytes=7288) at malloc.c:2891
#3  0x00007ffff7984d8f in ?? () from /usr/lib/x86_64-linux-gnu/libcurl.so.4
#4  0x00007ffff7984ecf in ?? () from /usr/lib/x86_64-linux-gnu/libcurl.so.4
#5  0x00007ffff7985cbd in ?? () from /usr/lib/x86_64-linux-gnu/libcurl.so.4
#6  0x0000000000506a08 in HTTPP::HTTP::client::detail::Manager::Manager
(this=0x811eb0, io=..., dispatch=...) at
/home/daedric/perso/httpp/src/httpp/http/client/Manager.cpp:52
#7  0x00000000004ef61b in HTTPP::HttpClient::HttpClient (this=0x7fffffffcfb0,
nb_thread=1, name="") at /home/daedric/perso/httpp/src/httpp/HttpClient.cpp:38

The segfault on the call of curl_multi_init and this one occurs in the
constructor. I was not able to check with valgrind so far if a failed
instanciation occurs after a trace about an invalid call_once/pthread_once

[Bug libstdc++/60966] std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #6 from Thomas Sanchez <thomas.sanchz at gmail dot com> ---
Last comment on myself:

I got some errors relates to the promise with valgrind but they do not cause an
hangs

==30999== Thread 2:
==30999== Invalid read of size 4
==30999==    at 0x4E44A91: pthread_once (pthread_once.S:111)
==30999==    by 0x4F4274: __gthread_once(int*, void (*)()) (gthr-default.h:699)
==30999==    by 0x4FADBC: void std::call_once<void
(std::__future_base::_State_base::*)(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>&, bool&),
std::__future_base::_State_base* const,
std::reference_wrapper<std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()> >, std::reference_wrapper<bool>
>(std::once_flag&, void
(std::__future_base::_State_base::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>&, bool&),
std::__future_base::_State_base* const&&,
std::reference_wrapper<std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()> >&&,
std::reference_wrapper<bool>&&) (mutex:786)
==30999==    by 0x4F75A2:
std::__future_base::_State_base::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base,
std::__future_base::_Result_base::_Deleter> ()>, bool) (future:358)
==30999==    by 0x50B666: std::promise<void>::set_value() (future:1197)
==30999==    by 0x506E1F:
HTTPP::HTTP::client::detail::Manager::check_handles(std::promise<void>&)
(Manager.cpp:87)
==30999==    by 0x51D9A6: void std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>::operator()<std::promise<void>&,
void>(HTTPP::HTTP::client::detail::Manager*, std::promise<void>&) const (in
/home/daedric/perso/httpp/build/tests/client/Test_Client_promise)
==30999==    by 0x51CFBE: void std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>::__call<void, , 0ul,
1ul>(std::tuple<>&&, std::_Index_tuple<0ul, 1ul>) (functional:1296)
==30999==    by 0x51C4FF: void std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>::operator()<, void>()
(functional:1355)
==30999==    by 0x51B594: void
boost::asio::asio_handler_invoke<std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)> >(std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>, ...)
(handler_invoke_hook.hpp:64)
==30999==    by 0x519E81: void
boost_asio_handler_invoke_helpers::invoke<std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>, std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)> >(std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>&, std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>&) (handler_invoke_helpers.hpp:37)
==30999==    by 0x51B6CE:
boost::asio::detail::completion_handler<std::_Bind<std::_Mem_fn<void
(HTTPP::HTTP::client::detail::Manager::*)(std::promise<void>&)>
(HTTPP::HTTP::client::detail::Manager*,
std::reference_wrapper<std::promise<void> >)>
>::do_complete(boost::asio::detail::task_io_service*,
boost::asio::detail::task_io_service_operation*, boost::system::error_code
const&, unsigned long) (completion_handler.hpp:68)
==30999==  Address 0xdbd3b54 is 132 bytes inside a block of size 136 free'd
==30999==    at 0x4C2C2BC: operator delete(void*) (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30999==    by 0x504C41:
__gnu_cxx::new_allocator<std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>
>::deallocate(std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>*,
unsigned long) (new_allocator.h:110)
==30999==    by 0x5045F2:
std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2> >
>::deallocate(std::allocator<std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>
>&, std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>, (__gnu_cxx::_Lock_policy)2>*,
unsigned long) (alloc_traits.h:377)
==30999==    by 0x505D85:
std::_Sp_counted_ptr_inplace<std::__future_base::_State_base,
std::allocator<std::__future_base::_State_base>,
(__gnu_cxx::_Lock_policy)2>::_M_destroy() (shared_ptr_base.h:417)
==30999==    by 0x4F2A51:
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release()
(shared_ptr_base.h:161)
==30999==    by 0x4F1CC4:
std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
(shared_ptr_base.h:546)
==30999==    by 0x4F15F3: std::__shared_ptr<std::__future_base::_State_base,
(__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:781)
==30999==    by 0x4F160D:
std::shared_ptr<std::__future_base::_State_base>::~shared_ptr()
(shared_ptr.h:93)
==30999==    by 0x50B491: std::promise<void>::~promise() (future:1136)
==30999==    by 0x5071C3: HTTPP::HTTP::client::detail::Manager::~Manager()
(Manager.cpp:121)
==30999==    by 0x4F2F2B:
std::default_delete<HTTPP::HTTP::client::detail::Manager>::operator()(HTTPP::HTTP::client::detail::Manager*)
const (unique_ptr.h:67)
==30999==    by 0x4F262F: std::unique_ptr<HTTPP::HTTP::client::detail::Manager,
std::default_delete<HTTPP::HTTP::client::detail::Manager>
>::reset(HTTPP::HTTP::client::detail::Manager*) (unique_ptr.h:262)

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Thomas Sanchez from comment #4)
> Anyway, I pushed the actual code in this branch:
> https://github.com/daedric/httpp/tree/bug-promise-test
> 
> I was able to do a small test that can *sometimes* reproduce the bug:
> https://github.com/daedric/httpp/blob/bug-promise-test/tests/client/promise.
> cpp

Great, thanks, I will look into it.

[Bug libstdc++/60966] std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #8 from Thomas Sanchez <thomas.sanchz at gmail dot com> ---
Hey,
I just wanted to know if you had the time to look into and/or if you were able
to reproduce the bug ?

Thanks,

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
No, not yet.

[Bug libstdc++/60966] std::call_once sometime hangs

[fawaka at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

Leon Timmermans <fawaka at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fawaka at gmail dot com

--- Comment #10 from Leon Timmermans <fawaka at gmail dot com> ---
(In reply to Thomas Sanchez from comment #8)
> Hey,
> I just wanted to know if you had the time to look into and/or if you were
> able to reproduce the bug ?
> 
> Thanks,

It seems I've hit exactly the same issue in my code: std::promise::set_value
hangs in pthread_once.

Interestingly, it always happens in the second thread of that type (same thread
function). I'm not sure how to interpret that.

Leon

[Bug libstdc++/60966] std::call_once sometime hangs

[hideaki.kimura at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

Hideaki Kimura <hideaki.kimura at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hideaki.kimura at gmail dot com

--- Comment #11 from Hideaki Kimura <hideaki.kimura at gmail dot com> ---
Hi, I'm also (seemingly) hitting this issue.
When I run my program with valgrind, I get what Thomas reported.

==22319== Invalid read of size 4
==22319==    at 0x370940D201: pthread_once (pthread_once.S:111)
==22319==    by 0x4C80524: ....<my functions> (gthr-default.h:699)
...
==22319==  Address 0x52c52a4 is 132 bytes inside a block of size 136 free'd
==22319==    at 0x4A07991: operator delete(void*) (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22319==    by 0x4C82456: ...<my function> (shared_ptr_base.h:161)
==22319==    by 0x370C4B52AF: execute_native_thread_routine (thread.cc:84)
==22319==    by 0x3709407F32: start_thread (pthread_create.c:309)
==22319==    by 0x37090F4DEC: clone (clone.S:111)
==22319== 

My environment is Fedora 20, x86_64, g++ (GCC) 4.8.2 20131212 (Red Hat
4.8.2-7).
Any workaround?

[Bug libstdc++/60966] std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #12 from Thomas Sanchez <thomas.sanchz at gmail dot com> ---
Hi Hideaki,

I was able to workaround the bug by using boost promise instead of the STL one.
I did not have the time to investigate the bug however, I'm not really familiar
with the STL design :(

Here is what I needed to do:

https://github.com/daedric/httpp/commit/b7d9a36c7a9fe9fdaed1771326c1c5e5eaaa507c

I did not hit the problem again so far.

Good luck !

Are you able to produce a smaller code which trigger the bug than I was?

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Hideaki Kimura from comment #11)
> Hi, I'm also (seemingly) hitting this issue.
> When I run my program with valgrind, I get what Thomas reported.
> 
> ==22319== Invalid read of size 4
> ==22319==    at 0x370940D201: pthread_once (pthread_once.S:111)
> ==22319==    by 0x4C80524: ....<my functions> (gthr-default.h:699)
> ...
> ==22319==  Address 0x52c52a4 is 132 bytes inside a block of size 136 free'd
> ==22319==    at 0x4A07991: operator delete(void*) (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==22319==    by 0x4C82456: ...<my function> (shared_ptr_base.h:161)
> ==22319==    by 0x370C4B52AF: execute_native_thread_routine (thread.cc:84)
> ==22319==    by 0x3709407F32: start_thread (pthread_create.c:309)
> ==22319==    by 0x37090F4DEC: clone (clone.S:111)
> ==22319== 

This means you are waiting on an object that has gone out of scope. WIthout
more information it's not possible to tell if this is a bug in your program or
the standard libary.

I'll try to reproduce it with Thomas's code...

[Bug libstdc++/60966] std::call_once sometime hangs

[hideaki.kimura at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #14 from Hideaki Kimura <hideaki.kimura at gmail dot com> ---
(In reply to Jonathan Wakely from comment #13)
> This means you are waiting on an object that has gone out of scope. WIthout
> more information it's not possible to tell if this is a bug in your program
> or the standard libary.
> 
> I'll try to reproduce it with Thomas's code...

Hey Jonathan,
I'm not sure if this helps, but could you try the following code snippet?

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

struct DummyTask {
    DummyTask(int id) : id_(id) {}
    int id_;
    std::promise<void> pr;
};

const int THREADS = 100;

void run_task(DummyTask* task) {
    std::this_thread::sleep_for(std::chrono::milliseconds(100));
    task->pr.set_value();
}

void wait_for_task(DummyTask* task) {
    task->pr.get_future().wait();
}

int main() {
    std::vector<DummyTask*> tasks;
    std::vector<std::thread*> threads;
    for (int i = 0; i < THREADS; ++i) {
        DummyTask* task = new DummyTask(i);
        tasks.push_back(task);
        threads.push_back(new std::thread(run_task, task));
    }

    for (int i = 0; i < THREADS; ++i) {
        wait_for_task(tasks[i]);
        // Because we returned from wait_for_task for this task, run_task is
surely done.
        // No one else is referring to the task. So, even before
threads[i]->join(),
        // it should be safe to delete it now.
        delete tasks[i];  // but here you get an invalid read!
    }
    for (int i = 0; i < THREADS; ++i) {
        threads[i]->join();
        delete threads[i];
    }
    return 0;
}


Run it a few times on valgrind. You will see what I got.
If you move threads[i]->join() to the line before delete tasks[i], you don't
get the issue.

[Assuming that I'm not terribly missing something..]
My bet is that std::promise puts something on thread-local, which causes some
issue when std::promise's destructor is called before the corresponding
thread's join() is called.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Hideaki Kimura from comment #14)
> I'm not sure if this helps, but could you try the following code snippet?

Very helpful, thanks

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Hideaki Kimura from comment #14)
> void run_task(DummyTask* task) {
>     std::this_thread::sleep_for(std::chrono::milliseconds(100));
>     task->pr.set_value();
> }
> 
> void wait_for_task(DummyTask* task) {
>     task->pr.get_future().wait();

There's a race condition here between the calls to get_future() and
set_value(), so the program has undefined behaviour.

promise::get_future() is a non-const function that modifies the promise object,
therefore it must not be called while any other object is accessing the
promise.

The easiest fix is to call get_future() before passing the task into a new
thread, and store it in a std::vector<std::future<void>>

If I do that I can't make it hang any longer.

I'll keep looking into it though ...

[Bug libstdc++/60966] std::call_once sometime hangs

[hideaki.kimura at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #17 from Hideaki Kimura <hideaki.kimura at gmail dot com> ---
(In reply to Jonathan Wakely from comment #16)
> promise::get_future() is a non-const function that modifies the promise
> object, therefore it must not be called while any other object is accessing
> the promise.
Oh, is it the design of promise::get_future/set_value?
I so far don't see any reference that clarifies either way (most document just
mentions about difference between future/shared_future)

Taking a glance at boost::promise code, I got an impression that their code is
safe against concurrent get_future() and set_value() because get_future()
either does atomic swap in lazy_init() or does nothing (allocated in
constructor).
I guess that's why Thomas observed that the issue doesn't happen in
boost::promise.

I'm not sure what the C++ standard says. I haven't seen a single human being
who has read it through.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #18 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Hideaki Kimura from comment #17)
> Oh, is it the design of promise::get_future/set_value?
> I so far don't see any reference that clarifies either way (most document
> just mentions about difference between future/shared_future)

It's the same rule to applies to all types in the standard library unless
specified otherwise: concurrent accesses are only safe if they are all reads
(i.e. const operations). Any writes (non-const operations) must be manually
synchronised.

get_future() is non-const, set_value() is non-const.

> I'm not sure what the C++ standard says. I haven't seen a single human being
> who has read it through.

30.6.4 [futures.state] says which functions are synchronised,
promise::get_future() is not one of them.

I'm still looking into whether there is a bug that allows the shared state to
be destroyed while the call to std::call_once() is still in progress, or if
that can only happen due to user error.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #19 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #16)
> The easiest fix is to call get_future() before passing the task into a new
> thread, and store it in a std::vector<std::future<void>>

Actually this only hides the error (by ensuring the shared state is not deleted
because there is a future object still referring to it) but the fundamental
problem with that code remains:

You are calling the promise destructor before the call to set_value()
completes.

You are assuming that as soon as the shared state becomes ready the promise is
no longer in use, but that's not true. After the shared state is made ready the
rest of the set_value() function runs, which accesses members of the shared
state. If you destroy the promise (and it has the only reference to the shared
state) then it will destroy its members while they are still being used.

This is a bug in your code, std::promise is like any other type: you must not
delete it while another thread is still using it.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-05-15
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #20 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I see an easy way to make std::promise handle this form of user error though.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #21 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Created attachment 32801
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=32801&action=edit
increase reference count on shared state during set_value / set_exception

This patch ensures the shared state will not be destroyed even if ~promise()
starts before the return from set_value.

It also moves the condition variables broadcast later, so that waiting threads
are not signalled until right before returning.

[Bug libstdc++/60966] std::call_once sometime hangs

[hideaki.kimura at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #22 from Hideaki Kimura <hideaki.kimura at gmail dot com> ---
Ah, you are right, set_value() might have context switch after signaling before
exitting.

... ah, and that's why what Thomas initially posted could also see a hang.
{ // copy-paste again to refresh memory, with some simplification
std::promise<void> promise;
auto future = promise.get_future();
std::async or thread ( {promise.set_value();});
future.get();
}

The user of std::promise must make sure the asynchronous thread has surely
exitted at least from promise.set_value() before std::promise gets out of
scope.
promise/future is a handy way to synchronize between threads, but to use it one
must protect it with thread synchronization..? That's quite tedious.


I've just seen the patch as of writing this comment. I guess it solves the
issue?

(In reply to Jonathan Wakely from comment #19)
> (In reply to Jonathan Wakely from comment #16)
> > The easiest fix is to call get_future() before passing the task into a new
> > thread, and store it in a std::vector<std::future<void>>
> 
> Actually this only hides the error (by ensuring the shared state is not
> deleted
> because there is a future object still referring to it) but the fundamental
> problem with that code remains:
> 
> You are calling the promise destructor before the call to set_value()
> completes.
> 
> You are assuming that as soon as the shared state becomes ready the promise
> is no longer in use, but that's not true. After the shared state is made
> ready the rest of the set_value() function runs, which accesses members of
> the shared state. If you destroy the promise (and it has the only reference
> to the shared state) then it will destroy its members while they are still
> being used.
> 
> This is a bug in your code, std::promise is like any other type: you must
> not delete it while another thread is still using it.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #23 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Hideaki Kimura from comment #22)
> Ah, you are right, set_value() might have context switch after signaling
> before exitting.

Or even before signalling (a waiting thread could check if the shared state is
ready between the value being set and the condvar signal)

> ... ah, and that's why what Thomas initially posted could also see a hang.
> { // copy-paste again to refresh memory, with some simplification
> std::promise<void> promise;
> auto future = promise.get_future();
> std::async or thread ( {promise.set_value();});
> future.get();
> }
> 
> The user of std::promise must make sure the asynchronous thread has surely
> exitted at least from promise.set_value() before std::promise gets out of
> scope.
> promise/future is a handy way to synchronize between threads, but to use it
> one must protect it with thread synchronization..? That's quite tedious.

If you give references to local variables to other threads then you need to be
careful about lifetimes, but this code is reasonable and should work.

> I've just seen the patch as of writing this comment. I guess it solves the
> issue?

Yes, it should solve the problem, by incrementing the reference count on the
shared state temporarily, until the set_value() call has completed.

[Bug libstdc++/60966] std::call_once sometime hangs

[fawaka at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #24 from Leon Timmermans <fawaka at gmail dot com> ---
(In reply to Jonathan Wakely from comment #18)
> (In reply to Hideaki Kimura from comment #17)
> > Oh, is it the design of promise::get_future/set_value?
> > I so far don't see any reference that clarifies either way (most document
> > just mentions about difference between future/shared_future)
> 
> It's the same rule to applies to all types in the standard library unless
> specified otherwise: concurrent accesses are only safe if they are all reads
> (i.e. const operations). Any writes (non-const operations) must be manually
> synchronised.
> 
> get_future() is non-const, set_value() is non-const.

I can see your point from a C++ point of view, but this doesn't make sense from
a usable threading point of view. IMHO, the whole point of abstractions such as
promises is to isolate the user from such issues.

Leon

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #25 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Leon Timmermans from comment #24)
> > get_future() is non-const, set_value() is non-const.
> 
> I can see your point from a C++ point of view, but this doesn't make sense
> from a usable threading point of view. IMHO, the whole point of abstractions
> such as promises is to isolate the user from such issues.

Within reason yes, but not entirely. You can't expect to safely assign to the
same std::promise in two threads for example.

Anyway, that's not the real issue here, the example can be fixed to avoid such
race conditions but still demonstrate the problem:

#include <future>
#include <thread>
#include <vector>

struct DummyTask {
    DummyTask(int id) : id_(id) {}
    int id_;
    std::promise<void> pr;
};

const int THREADS = 100;

void run_task(DummyTask* task) {
    std::this_thread::sleep_for(std::chrono::milliseconds(100));
    task->pr.set_value();
}

void wait_for_task(std::future<void> fut) {
    fut.wait();
}

int main() {
    std::vector<const DummyTask*> tasks;
    std::vector<std::thread> threads;
    std::vector<std::future<void>> futures;
    for (int i = 0; i < THREADS; ++i) {
        DummyTask* task = new DummyTask(i);
        tasks.push_back(task);
        futures.push_back(task->pr.get_future());
        threads.emplace_back(run_task, task);
    }

    for (int i = 0; i < THREADS; ++i) {
        wait_for_task(std::move(futures[i]));
        // Because we returned from wait_for_task for this task, run_task is
surely done.
        // No one else is referring to the task. So, even before
threads[i]->join(),
        // it should be safe to delete it now.
        delete tasks[i];  // but here you get an invalid read!
    }
    for (int i = 0; i < THREADS; ++i) {
        threads[i].join();
    }
}

[Bug libstdc++/60966] std::call_once sometime hangs

[thomas.sanchz at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #26 from Thomas Sanchez <thomas.sanchz at gmail dot com> ---
In the end the problem is quite simple :) One workaround would be to move the
promise into the lambda however I would be glad that your patch get accepted,
because IMHO it is not an expected behavior from a user point of view !

Thanks a lot for your work !

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #27 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Sat May 17 12:58:46 2014
New Revision: 210556

URL: http://gcc.gnu.org/viewcvs?rev=210556&root=gcc&view=rev
Log:
    PR libstdc++/60966
    * include/std/future (__future_base::_State_baseV2::_M_set_result):
    Pass lock into _M_do_set and hold it until the function returns.
    Signal condition variable after call_once completes.
    (__future_base::_State_baseV2::_M_do_set): Use lock argument. Do not
    signal here.
    * testsuite/30_threads/promise/60966.cc: New.

Added:
    trunk/libstdc++-v3/testsuite/30_threads/promise/60966.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/future

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #28 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Sat May 17 13:01:11 2014
New Revision: 210557

URL: http://gcc.gnu.org/viewcvs?rev=210557&root=gcc&view=rev
Log:
    PR libstdc++/60966
    * include/std/future (__future_base::_State_baseV2::_M_set_result):
    Signal condition variable after call_once returns.
    (__future_base::_State_baseV2::_M_do_set): Do not signal here.
    (promise::set_value, promise::set_exception): Increment the reference
    count on the shared state until the function returns.
    * testsuite/30_threads/promise/60966.cc: New.

Added:
    branches/gcc-4_9-branch/libstdc++-v3/testsuite/30_threads/promise/60966.cc
Modified:
    branches/gcc-4_9-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_9-branch/libstdc++-v3/include/std/future

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.4
      Known to fail|                            |4.8.2, 4.9.0

--- Comment #29 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk and 4.9 branch so far.

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #30 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Tue Jun  3 17:58:56 2014
New Revision: 211198

URL: http://gcc.gnu.org/viewcvs?rev=211198&root=gcc&view=rev
Log:
Backport from mainline
2014-05-16  Jonathan Wakely  <jwakely@redhat.com>

    PR libstdc++/60966
    * include/std/future (__future_base::_State_baseV2::_M_set_result):
    Signal condition variable after call_once returns.
    (__future_base::_State_baseV2::_M_do_set): Do not signal here.
    (promise::set_value, promise::set_exception): Increment the reference
    count on the shared state until the function returns.
    * testsuite/30_threads/promise/60966.cc: New.

Added:
    branches/gcc-4_8-branch/libstdc++-v3/testsuite/30_threads/promise/60966.cc
Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/include/std/future

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #31 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 4.8.4 and 4.9.1

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #32 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Fri Jan  9 14:40:16 2015
New Revision: 219393

URL: https://gcc.gnu.org/viewcvs?rev=219393&root=gcc&view=rev
Log:
    PR libstdc++/60966
    * include/std/future (packaged_task::operator()): Increment the
    reference count on the shared state until the function returns.

Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/include/std/future

[Bug libstdc++/60966] std::call_once sometime hangs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60966

--- Comment #33 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Fri Jan  9 14:40:46 2015
New Revision: 219394

URL: https://gcc.gnu.org/viewcvs?rev=219394&root=gcc&view=rev
Log:
    PR libstdc++/60966
    * include/std/future (packaged_task::operator()): Increment the
    reference count on the shared state until the function returns.

Modified:
    branches/gcc-4_9-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_9-branch/libstdc++-v3/include/std/future