1. 22 Nov, 2019 1 commit
    • 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
  2. 14 Nov, 2019 2 commits