Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix address sanitizer issue in new_worker (rx-newthread.hpp) #497

Merged
merged 1 commit into from
Apr 26, 2019

Conversation

luckychess
Copy link
Contributor

Greetings!
Some time ago I performed checks of some project which uses RxCpp with address sanitizer and got this:

==87760==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000002810 at pc 0x000107a5ccd8 bp 0x70000f5b8dd0 sp 0x70000f5b8580
READ of size 8 at 0x60d000002810 thread T7
    #0 0x107a5ccd7 in __asan_memcpy (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x50cd7)
    #1 0x105c0b181 in std::__1::cv_status std::__1::condition_variable::wait_until<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >(std::__1::unique_lock<std::__1::mutex>&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > const&) chrono:859
    #2 0x105c07d02 in rxcpp::schedulers::new_thread::new_worker::new_worker(rxcpp::composite_subscription, std::__1::function<std::__1::thread (std::__1::function<void ()>)>&)::'lambda0'()::operator()() const rx-newthread.hpp:113
    #3 0x105c06b5c in _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN5rxcpp10schedulers10new_thread10new_workerC1ENS3_22composite_subscriptionERNS_8functionIFNS_6threadENS8_IFvvEEEEEEEUlvE0_EEEvDpOT_ type_traits:4291
    #4 0x105c06558 in std::__1::__function::__func<rxcpp::schedulers::new_thread::new_worker::new_worker(rxcpp::composite_subscription, std::__1::function<std::__1::thread (std::__1::function<void ()>)>&)::'lambda0'(), std::__1::allocator<rxcpp::schedulers::new_thread::new_worker::new_worker(rxcpp::composite_subscription, std::__1::function<std::__1::thread (std::__1::function<void ()>)>&)::'lambda0'()>, void ()>::operator()() functional:1552
    #5 0x105be30ff in std::__1::function<void ()>::operator()() const functional:1903
    #6 0x105be2b17 in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::function<void ()> > >(void*) type_traits:4291
    #7 0x7fff5081b660 in _pthread_body (libsystem_pthread.dylib:x86_64+0x3660)
    #8 0x7fff5081b50c in _pthread_start (libsystem_pthread.dylib:x86_64+0x350c)
    #9 0x7fff5081abf8 in thread_start (libsystem_pthread.dylib:x86_64+0x2bf8)

0x60d000002810 is located 0 bytes inside of 144-byte region [0x60d000002810,0x60d0000028a0)
freed by thread T5 here:
    #0 0x107a706ab in wrap__ZdlPv (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x646ab)
    #1 0x105c1f1aa in std::__1::__split_buffer<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long>, std::__1::allocator<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long> >&>::~__split_buffer() new:234
    #2 0x105c1df14 in std::__1::__split_buffer<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long>, std::__1::allocator<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long> >&>::~__split_buffer() __split_buffer:340
    #3 0x105c1bc9c in void std::__1::vector<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long>, std::__1::allocator<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long> > >::__push_back_slow_path<std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long> >(std::__1::pair<rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >, long long>&&) vector:1575
    #4 0x105c1a8ec in rxcpp::schedulers::detail::schedulable_queue<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >::push(rxcpp::schedulers::detail::time_schedulable<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >&&) vector:1611
    #5 0x105be906d in rxcpp::schedulers::new_thread::new_worker::schedule(std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, rxcpp::schedulers::schedulable const&) const rx-newthread.hpp:136
    #6 0x105be8b7a in rxcpp::schedulers::new_thread::new_worker::schedule(rxcpp::schedulers::schedulable const&) const rx-newthread.hpp:130

Logs are truncated a bit, please check https://gist.github.com/luckychess/0abced67530eda0ede1456e666d2ccf6 for details.
After some investigation the problem became more clear to me:

  • Thread 1 gets a reference to some item in a queue (line 107) and passes it to wait_until (line 113)
  • Call to wait_until releases the lock
  • Thread 2 calls push (line 136)
    And after that the reference becomes invalid according to ASAN, because of a data race in an underlying queue.
    My propose is to retrieve a copy of a queue item instead of reference. It eliminates ASAN errors.
    Also ASAN blames for this only under MacOS but finds no issues on Linux. Probably because of a different queue implementation.

@luckychess
Copy link
Contributor Author

Another possibility suggested by @lebdron is to create a separate variable for peek.when.

@kirkshoop
Copy link
Member

Yes, I would like to have a copy of peek.when and keep peek a reference

Signed-off-by: Konstantin Munichev <toobwn@gmail.com>
@luckychess
Copy link
Contributor Author

@kirkshoop okay, I did a change

@kirkshoop kirkshoop merged commit f38fb8e into ReactiveX:master Apr 26, 2019
@kirkshoop
Copy link
Member

Thank you!

@luckychess luckychess deleted the fix/asan_new_worker branch April 29, 2019 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants