• Kirill Smelkov's avatar
    libgolang: Fix select crash while accessing destroyed channel upon wakeup · 5aa1e899
    Kirill Smelkov authored
    Similarly to situation described in dcf4ebd1 (libgolang: Fix chan.close
    to dequeue subscribers atomically), select can be also accessing a
    channel object at the time of wakeup when that channel could be already
    destroyed: select queues waiters to channels recv/send queues and upon
    wakeup needs to dequeue them. This requires locking channels, not to
    mention that a channel destroy with non-empty subscribers queue will
    trigger bug panic.
    
    Contrary to the fix for recv, we cannot rework select not to access
    channel objects after wakeup, because for select upon wakeup all queued
    channels could be already destroyed, not only selected one. Thus the fix
    here is to incref/decref the channels for the duration where we need to
    access them.
    
    The bug was not caught by existing tests and was noted while doing
    libgolang.cpp review for concurrency issues. With added test (hereby fix
    is served by a bit amended _test_close_wakeup_all) the bug, if not
    fixed, renders itself as e.g. the following under TSAN:
    
    WARNING: ThreadSanitizer: data race (pid=4421)
      Write of size 8 at 0x7b1400000650 by thread T9:
        #0 free ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:649 (libtsan.so.0+0x2b46a)
        #1 free ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:643 (libtsan.so.0+0x2b46a)
        #2 golang::_chan::decref() golang/runtime/libgolang.cpp:479 (liblibgolang.so.0.1+0x4822)
        #3 _chanxdecref golang/runtime/libgolang.cpp:461 (liblibgolang.so.0.1+0x487a)
        #4 golang::chan<int>::operator=(decltype(nullptr)) golang/libgolang.h:296 (_golang_test.so+0x14cf9)
        #5 operator() golang/runtime/libgolang_test.cpp:304 (_golang_test.so+0x14cf9)
        #6 __invoke_impl<void, __test_close_wakeup_all(bool)::<lambda()>&> /usr/include/c++/8/bits/invoke.h:60 (_golang_test.so+0x14cf9)
        #7 __invoke<__test_close_wakeup_all(bool)::<lambda()>&> /usr/include/c++/8/bits/invoke.h:95 (_golang_test.so+0x14cf9)
        #8 __call<void> /usr/include/c++/8/functional:400 (_golang_test.so+0x14cf9)
        #9 operator()<> /usr/include/c++/8/functional:484 (_golang_test.so+0x14cf9)
        #10 _M_invoke /usr/include/c++/8/bits/std_function.h:297 (_golang_test.so+0x14cf9)
        #11 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (_golang_test.so+0x1850c)
        #12 operator() golang/libgolang.h:273 (_golang_test.so+0x1843a)
        #13 _FUN golang/libgolang.h:271 (_golang_test.so+0x1843a)
        #14 <null> <null> (python2.7+0x1929e3)
    
      Previous read of size 8 at 0x7b1400000650 by thread T10:
        #0 golang::Sema::acquire() golang/runtime/libgolang.cpp:168 (liblibgolang.so.0.1+0x413a)
        #1 golang::Mutex::lock() golang/runtime/libgolang.cpp:179 (liblibgolang.so.0.1+0x424a)
        #2 operator() golang/runtime/libgolang.cpp:1044 (liblibgolang.so.0.1+0x424a)
        #3 _M_invoke /usr/include/c++/8/bits/std_function.h:297 (liblibgolang.so.0.1+0x424a)
        #4 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (liblibgolang.so.0.1+0x5f07)
        #5 golang::_deferred::~_deferred() golang/runtime/libgolang.cpp:215 (liblibgolang.so.0.1+0x5f07)
        #6 __chanselect2 golang/runtime/libgolang.cpp:1044 (liblibgolang.so.0.1+0x5f07)
        #7 _chanselect2<true> golang/runtime/libgolang.cpp:968 (liblibgolang.so.0.1+0x6665)
        #8 _chanselect golang/runtime/libgolang.cpp:963 (liblibgolang.so.0.1+0x6665)
        #9 select golang/libgolang.h:386 (_golang_test.so+0x14fc1)
        #10 operator() golang/runtime/libgolang_test.cpp:320 (_golang_test.so+0x14fc1)
        #11 __invoke_impl<void, __test_close_wakeup_all(bool)::<lambda()>&> /usr/include/c++/8/bits/invoke.h:60 (_golang_test.so+0x14fc1)
        #12 __invoke<__test_close_wakeup_all(bool)::<lambda()>&> /usr/include/c++/8/bits/invoke.h:95 (_golang_test.so+0x14fc1)
        #13 __call<void> /usr/include/c++/8/functional:400 (_golang_test.so+0x14fc1)
        #14 operator()<> /usr/include/c++/8/functional:484 (_golang_test.so+0x14fc1)
        #15 _M_invoke /usr/include/c++/8/bits/std_function.h:297 (_golang_test.so+0x14fc1)
        #16 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (_golang_test.so+0x1850c)
        #17 operator() golang/libgolang.h:273 (_golang_test.so+0x183da)
        #18 _FUN golang/libgolang.h:271 (_golang_test.so+0x183da)
        #19 <null> <null> (python2.7+0x1929e3)
    
      Thread T9 (tid=4661, running) created by main thread at:
        #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:915 (libtsan.so.0+0x2be1b)
        #1 PyThread_start_new_thread <null> (python2.7+0x19299f)
        #2 _taskgo golang/runtime/libgolang.cpp:123 (liblibgolang.so.0.1+0x3f98)
        #3 go<__test_close_wakeup_all(bool)::<lambda()> > golang/libgolang.h:271 (_golang_test.so+0x16c94)
        #4 __test_close_wakeup_all(bool) golang/runtime/libgolang_test.cpp:298 (_golang_test.so+0x16c94)
        #5 _test_close_wakeup_all_vsselect() golang/runtime/libgolang_test.cpp:342 (_golang_test.so+0x16f64)
        #6 __pyx_pf_6golang_12_golang_test_24test_close_wakeup_all_vsselect golang/_golang_test.cpp:4013 (_golang_test.so+0xd92a)
        #7 __pyx_pw_6golang_12_golang_test_25test_close_wakeup_all_vsselect golang/_golang_test.cpp:3978 (_golang_test.so+0xd92a)
        #8 PyEval_EvalFrameEx <null> (python2.7+0xf68b4)
    
      Thread T10 (tid=4662, running) created by main thread at:
        #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:915 (libtsan.so.0+0x2be1b)
        #1 PyThread_start_new_thread <null> (python2.7+0x19299f)
        #2 _taskgo golang/runtime/libgolang.cpp:123 (liblibgolang.so.0.1+0x3f98)
        #3 go<__test_close_wakeup_all(bool)::<lambda()> > golang/libgolang.h:271 (_golang_test.so+0x16d96)
        #4 __test_close_wakeup_all(bool) golang/runtime/libgolang_test.cpp:315 (_golang_test.so+0x16d96)
        #5 _test_close_wakeup_all_vsselect() golang/runtime/libgolang_test.cpp:342 (_golang_test.so+0x16f64)
        #6 __pyx_pf_6golang_12_golang_test_24test_close_wakeup_all_vsselect golang/_golang_test.cpp:4013 (_golang_test.so+0xd92a)
        #7 __pyx_pw_6golang_12_golang_test_25test_close_wakeup_all_vsselect golang/_golang_test.cpp:3978 (_golang_test.so+0xd92a)
        #8 PyEval_EvalFrameEx <null> (python2.7+0xf68b4)
    
    and reliably crashes under regular builds.
    5aa1e899
libgolang_test.cpp 11.4 KB