-
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