Closed hcho3 closed 5 years ago
Consider enable sanitizer in the CI after these fixes?
@jermainewang There's still a long way to go until we resolve all errors reported by the Sanitizer: #553
We can fix them one by one?
@trivialfis Yes, I plan to merge this PR pretty soon (after addressing comments from @junrushao1994), and then tackle remaining concurrency issues in subsequent PRs.
@trivialfis Since there are outstanding concurrency issues, I'm afraid we are not yet able to add sanitizer to CI.
@trivialfis For now, I added the sanitizer test and marked it non-blocking.
Fixes #550
Prevent "thread leak": Wrap
std::thread*
withstd::unique_ptr<>
andScopedThread
so that we can automaticallyjoin()
a thread whenever an exception is thrown. Do not use raw pointer to store the thread, as it's not exception-safe.Eliminate data races in producer signals: Make
producer_sig_processed_
andproducer_sig_
atomic because 1) Multiple threads access these variables; 2) The variables are often used without a lock.Furthermore, we use
.store(*, std::memory_order_release)
and.load(std::memory_order_acquire)
so that a write from one thread is guaranteed to be visible from another thread. See Release-Acquire Ordering.Enable building dmlc-core with Sanitizer. For example, to use Thread Sanitizer, run:
cc @larroy @jermainewang