skypjack / uvw

Header-only, event based, tiny and easy to use libuv wrapper in modern C++ - now available as also shared/static library!
MIT License
1.82k stars 207 forks source link

Segmentation fault on calling thread.join due to destructor also calling thread.join #292

Open wirepair opened 1 year ago

wirepair commented 1 year ago

Code to replicate:

int main(int argc, char *argv[]) 
{
    auto loop = uvw::loop::get_default();

    auto threadWorker = [](std::shared_ptr<void> d) {
        std::cout << "all done" << std::endl;
    };

    std::vector<std::shared_ptr<uvw::thread>> threads;
    for (int i = 0; i < 8; i++)
    {
        auto threadId = std::make_shared<int>(i);
        auto threadHandle = loop->resource<uvw::thread>(threadWorker, threadId);
        threads.push_back(threadHandle);
    }

    for (auto& threadHandle : threads) 
    {
        threadHandle->run();
    }

    loop->run();

    for (auto& threadHandle : threads) 
    {
        threadHandle->join();
    }
    return 0;
}

Took me a while to debug, and only occurred when i had > 4 threads (???) but I get a segmentation fault since join() is being called twice (once by me, and once on exiting the main function scope). I'm not sure how we can call join if it's also apart of the destructor?

Here's the backtrace:

Core was generated by `./pmoclient'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007fa698be1aab in __pthread_clockjoin_ex (threadid=0x7fa698682700, thread_return=0x0, clockid=0x0, abstime=0x0, block=0x1) at pthread_join_common.c:89
89      pthread_join_common.c: No such file or directory.
gef➤  bt
#0  0x00007fa698be1aab in __pthread_clockjoin_ex (threadid=0x7fa698682700, thread_return=0x0, clockid=0x0, abstime=0x0, block=0x1) at pthread_join_common.c:89
#1  0x00000000004813fc in uv_thread_join (tid=0x10351e8) at /home/wirepair/netcode/pmo/build/_deps/libuv-src/src/unix/thread.c:286
#2  0x000000000045f94d in uvw::thread::join (this=0x10351d0) at /home/wirepair/netcode/pmo/build/_deps/uvw-src/src/uvw/thread.cpp:45
#3  0x000000000045f8ec in uvw::thread::~thread (this=0x10351d0) at /home/wirepair/netcode/pmo/build/_deps/uvw-src/src/uvw/thread.cpp:32
#4  0x000000000040c549 in __gnu_cxx::new_allocator<uvw::thread>::destroy<uvw::thread> (this=0x10351d0, __p=0x10351d0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/ext/new_allocator.h:152
#5  0x000000000040c4e0 in std::allocator_traits<std::allocator<uvw::thread> >::destroy<uvw::thread> (__a=..., __p=0x10351d0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/alloc_traits.h:496
#6  0x000000000040bf8f in std::_Sp_counted_ptr_inplace<uvw::thread, std::allocator<uvw::thread>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x10351c0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/shared_ptr_base.h:557
#7  0x000000000040ab9c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x10351c0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/shared_ptr_base.h:155
#8  0x000000000040ab4a in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x10354f8) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/shared_ptr_base.h:730
#9  0x000000000040ca3e in std::__shared_ptr<uvw::thread, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x10354f0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/shared_ptr_base.h:1169
#10 0x000000000040a6e8 in std::shared_ptr<uvw::thread>::~shared_ptr (this=0x10354f0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/shared_ptr.h:103
#11 0x000000000040ad95 in std::_Destroy<std::shared_ptr<uvw::thread> > (__pointer=0x10354f0) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_construct.h:98
#12 0x000000000040ad5f in std::_Destroy_aux<false>::__destroy<std::shared_ptr<uvw::thread>*> (__first=0x10354f0, __last=0x1035570) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_construct.h:108
#13 0x000000000040ad1d in std::_Destroy<std::shared_ptr<uvw::thread>*> (__first=0x10354f0, __last=0x1035570) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_construct.h:136
#14 0x000000000040ac81 in std::_Destroy<std::shared_ptr<uvw::thread>*, std::shared_ptr<uvw::thread> > (__first=0x10354f0, __last=0x1035570) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_construct.h:206
#15 0x000000000040a84b in std::vector<std::shared_ptr<uvw::thread>, std::allocator<std::shared_ptr<uvw::thread> > >::~vector (this=0x7ffd2806d290) at /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_vector.h:677
#16 0x000000000040869a in main (argc=0x1, argv=0x7ffd2806d3c8) at /home/wirepair/netcode/pmo/apps/pmoclient.cpp:41
skypjack commented 1 year ago

🤔 I don't know very well this part of libuv, so bear with me. Is uv_thread_join idempotent? That is, can I invoke it multiple times on the same thread? I thought that was the case but I could be wrong and I can't find any reference to the libuv documentation. However, if this isn't the case, is there a function in libuv to know when a join is needed and when it isn't because the thread has already returned?

skypjack commented 1 year ago

CC @stefanofiorentino do you know how it works under the hood within libuv?

stefanofiorentino commented 1 year ago

CC @stefanofiorentino do you know how it works under the hood within libuv?

Let me check some internals..

stefanofiorentino commented 1 year ago

@wirepair as in uvw we are leveraging the RAII pattern, the destructor of the of uvw::thread is already calling join, so currently you could not call it by yourself. @skypjack we can either put join private or create an invariant to avoid the second call to join in case the public join has been called. I guess giving the user the freedom to force a join is the way to go, do you agree?

skypjack commented 1 year ago

Yeah, makes sense. A boolean member is enough for that probably. Not a big deal. 👍