Skip to content

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

Merged
kirkshoop merged 1 commit intoReactiveX:masterfrom
luckychess:fix/asan_new_worker
Apr 26, 2019
Merged

Fix address sanitizer issue in new_worker (rx-newthread.hpp)#497
kirkshoop merged 1 commit intoReactiveX:masterfrom
luckychess:fix/asan_new_worker

Conversation

@luckychess
Copy link
Copy Markdown
Contributor

@luckychess luckychess commented Apr 26, 2019

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
Copy Markdown
Contributor Author

luckychess commented Apr 26, 2019

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

@kirkshoop
Copy link
Copy Markdown
Member

kirkshoop commented Apr 26, 2019

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

@luckychess luckychess force-pushed the fix/asan_new_worker branch from c8741a5 to 07a06db Compare April 26, 2019 15:29
@luckychess
Copy link
Copy Markdown
Contributor Author

luckychess commented Apr 26, 2019

@kirkshoop okay, I did a change

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

kirkshoop commented Apr 26, 2019

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@luckychess @kirkshoop