• Kirill Smelkov's avatar
    libgolang: Fix globals atexit race condition of ~refptr vs access from another thread · fd2a6fab
    Kirill Smelkov authored
    Until now we were defining errors like this:
    
        const error ErrSomething  = errors::New("abc");
    
    However there is a problem here: the runtime will call ErrSomething
    destructor on program exit, and for refptr<T> the destructor does 2
    things: a) decrefs the object, and b) switches embedded pointer to NULL;
    
    If another thread is still running both "a" and "b" can become race
    conditions if that T2 uses ErrSomething via e.g. just
    
        ...
        if (err == ErrSomething)
            ...
    
    Here is, for example TSAN report about ErrPyStopped usage when accessed by timer thread:
    
        WARNING: ThreadSanitizer: data race (pid=4224)
         Read of size 8 at 0x7f0d6840e150 by thread T92:
           #0 golang::refptr<golang::_error>::refptr(golang::refptr<golang::_error> const&) golang/libgolang.h:525 (liblibpyxruntime.so.0.1+0x4edc)
           #1 golang::pyx::runtime::PyFunc::operator()() const golang/runtime/libpyxruntime.cpp:222 (liblibpyxruntime.so.0.1+0x423d)
           #2 std::_Function_handler<void (), golang::pyx::runtime::PyFunc>::_M_invoke(std::_Any_data const&) /usr/include/c++/8/bits/std_function.h:297 (_time.so+0x1382b)
           #3 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (liblibgolang.so.0.1+0x2ede6)
           #4 golang::time::_Timer::_fire(double, int) golang/time.cpp:197 (liblibgolang.so.0.1+0x48cbe)
           #5 operator() golang/time.cpp:171 (liblibgolang.so.0.1+0x4894c)
           #6 __invoke_impl<void, golang::time::_Timer::reset(double)::<lambda(int)>&, int&> /usr/include/c++/8/bits/invoke.h:60 (liblibgolang.so.0.1+0x4a142)
           #7 __invoke<golang::time::_Timer::reset(double)::<lambda(int)>&, int&> /usr/include/c++/8/bits/invoke.h:95 (liblibgolang.so.0.1+0x4a080)
           #8 __call<void, 0> /usr/include/c++/8/functional:400 (liblibgolang.so.0.1+0x49fcc)
           #9 operator()<> /usr/include/c++/8/functional:484 (liblibgolang.so.0.1+0x49cee)
           #10 _M_invoke /usr/include/c++/8/bits/std_function.h:297 (liblibgolang.so.0.1+0x49858)
           #11 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (liblibgolang.so.0.1+0x2ede6)
           #12 operator() golang/libgolang.h:326 (liblibgolang.so.0.1+0x48fa1)
           #13 _FUN golang/libgolang.h:324 (liblibgolang.so.0.1+0x49004)
           #14 <null> <null> (python2+0x194d13)
    
         Previous write of size 8 at 0x7f0d6840e150 by main thread:
           #0 golang::refptr<golang::_error>::~refptr() golang/libgolang.h:510 (liblibpyxruntime.so.0.1+0x4d39)
           #1 at_exit_wrapper ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:389 (libtsan.so.0+0x28693)
    
         As if synchronized via sleep:
           #0 nanosleep ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:366 (libtsan.so.0+0x49960)
           #1 __pyx_f_6golang_7runtime_15_runtime_thread_nanosleep golang/runtime/_runtime_thread.c:1702 (_runtime_thread.so+0x48f2)
           #2 _tasknanosleep golang/runtime/libgolang.cpp:1237 (liblibgolang.so.0.1+0x2cca7)
           #3 golang::time::sleep(double) golang/runtime/libgolang.cpp:1251 (liblibgolang.so.0.1+0x2ce0b)
           #4 golang::time::_Timer::_fire(double, int) golang/time.cpp:179 (liblibgolang.so.0.1+0x48bd6)
           #5 operator() golang/time.cpp:171 (liblibgolang.so.0.1+0x4894c)
           #6 __invoke_impl<void, golang::time::_Timer::reset(double)::<lambda(int)>&, int&> /usr/include/c++/8/bits/invoke.h:60 (liblibgolang.so.0.1+0x4a142)
           #7 __invoke<golang::time::_Timer::reset(double)::<lambda(int)>&, int&> /usr/include/c++/8/bits/invoke.h:95 (liblibgolang.so.0.1+0x4a080)
           #8 __call<void, 0> /usr/include/c++/8/functional:400 (liblibgolang.so.0.1+0x49fcc)
           #9 operator()<> /usr/include/c++/8/functional:484 (liblibgolang.so.0.1+0x49cee)
           #10 _M_invoke /usr/include/c++/8/bits/std_function.h:297 (liblibgolang.so.0.1+0x49858)
           #11 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (liblibgolang.so.0.1+0x2ede6)
           #12 operator() golang/libgolang.h:326 (liblibgolang.so.0.1+0x48fa1)
           #13 _FUN golang/libgolang.h:324 (liblibgolang.so.0.1+0x49004)
           #14 <null> <null> (python2+0x194d13)
    
         Location is global 'golang::pyx::runtime::ErrPyStopped' of size 8 at 0x7f0d6840e150 (liblibpyxruntime.so.0.1+0x000000009150)
    
         Thread T92 (tid=7834, running) created by thread T15 at:
           #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:915 (libtsan.so.0+0x2be1b)
           #1 PyThread_start_new_thread <null> (python2+0x194ccf)
           #2 _taskgo golang/runtime/libgolang.cpp:124 (liblibgolang.so.0.1+0x288e7)
           #3 go<golang::time::_Timer::reset(double)::<lambda(int)>, int> golang/libgolang.h:324 (liblibgolang.so.0.1+0x490b7)
           #4 golang::time::_Timer::reset(double) golang/time.cpp:170 (liblibgolang.so.0.1+0x48b36)
           #5 __pyx_f_6golang_5_time_timer_reset_pyexc golang/_time.cpp:3271 (_time.so+0xa506)
           #6 __pyx_pf_6golang_5_time_7PyTimer_6reset golang/_time.cpp:2824 (_time.so+0x97d8)
           #7 __pyx_pw_6golang_5_time_7PyTimer_7reset golang/_time.cpp:2789 (_time.so+0x9751)
           #8 PyEval_EvalFrameEx <null> (python2+0xf27f7)
           #9 std::_Function_handler<void (), golang::pyx::runtime::PyFunc>::_M_invoke(std::_Any_data const&) /usr/include/c++/8/bits/std_function.h:297 (_time.so+0x1382b)
           #10 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (liblibgolang.so.0.1+0x2ede6)
           #11 golang::time::_Timer::_fire(double, int) golang/time.cpp:197 (liblibgolang.so.0.1+0x48cbe)
           #12 operator() golang/time.cpp:171 (liblibgolang.so.0.1+0x4894c)
           #13 __invoke_impl<void, golang::time::_Timer::reset(double)::<lambda(int)>&, int&> /usr/include/c++/8/bits/invoke.h:60 (liblibgolang.so.0.1+0x4a142)
           #14 __invoke<golang::time::_Timer::reset(double)::<lambda(int)>&, int&> /usr/include/c++/8/bits/invoke.h:95 (liblibgolang.so.0.1+0x4a080)
           #15 __call<void, 0> /usr/include/c++/8/functional:400 (liblibgolang.so.0.1+0x49fcc)
           #16 operator()<> /usr/include/c++/8/functional:484 (liblibgolang.so.0.1+0x49cee)
           #17 _M_invoke /usr/include/c++/8/bits/std_function.h:297 (liblibgolang.so.0.1+0x49858)
           #18 std::function<void ()>::operator()() const /usr/include/c++/8/bits/std_function.h:687 (liblibgolang.so.0.1+0x2ede6)
           #19 operator() golang/libgolang.h:326 (liblibgolang.so.0.1+0x48fa1)
           #20 _FUN golang/libgolang.h:324 (liblibgolang.so.0.1+0x49004)
           #21 <null> <null> (python2+0x194d13)
    
        SUMMARY: ThreadSanitizer: data race golang/libgolang.h:525 in golang::refptr<golang::_error>::refptr(golang::refptr<golang::_error> const&)
    
    -> Fix it by not deallocating the object, nor clearing the pointer on smart pointer destruction.
    
    To do so in ergonomic way introduce another template type global<X>,
    which can be used instead of X and change types of all global error
    variables from `const error` to `const global<error>`. Don't change pxd
    to offload pyx users from thinking about global/not-global aspect.
    
    Top-level documentation is TODO.
    fd2a6fab
libpyxruntime.cpp 6.33 KB