Skip to content

Commit

Permalink
[core] Replace thread checker implementation (#48004)
Browse files Browse the repository at this point in the history
Signed-off-by: dentiny <dentinyhao@gmail.com>
  • Loading branch information
dentiny authored Nov 1, 2024
1 parent 9642919 commit ba41ae9
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 31 deletions.
1 change: 1 addition & 0 deletions src/ray/util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ cc_library(
strip_include_prefix = "//src",
visibility = ["//visibility:public"],
deps = [
":thread_checker",
"//:aligned_alloc",
"//:sha256",
"//src/ray/protobuf:event_cc_proto",
Expand Down
37 changes: 6 additions & 31 deletions src/ray/util/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "ray/util/logging.h"
#include "ray/util/macros.h"
#include "ray/util/process.h"
#include "ray/util/thread_checker.h"

#ifdef _WIN32
#include <process.h> // to ensure getpid() on Windows
Expand Down Expand Up @@ -331,54 +332,28 @@ class ThreadPrivate {
explicit ThreadPrivate(Ts &&...ts) : t_(std::forward<Ts>(ts)...) {}

T &operator*() {
ThreadCheck();
RAY_CHECK(thread_checker_.IsOnSameThread());
return t_;
}

T *operator->() {
ThreadCheck();
RAY_CHECK(thread_checker_.IsOnSameThread());
return &t_;
}

const T &operator*() const {
ThreadCheck();
RAY_CHECK(thread_checker_.IsOnSameThread());
return t_;
}

const T *operator->() const {
ThreadCheck();
RAY_CHECK(thread_checker_.IsOnSameThread());
return &t_;
}

private:
void ThreadCheck() const {
// ThreadCheck is not a thread safe function and at the same time, multiple
// threads might be accessing id_ at the same time.
// Here we only introduce mutex to protect write instead of read for the
// following reasons:
// - read and write at the same time for `id_` is fine since this is a
// trivial object. And since we are using this to detect errors,
// it doesn't matter which value it is.
// - read and write of `thread_name_` is not good. But it will only be
// read when we crash the program.
//
if (id_ == std::thread::id()) {
// Protect thread_name_
std::lock_guard<std::mutex> _(mutex_);
thread_name_ = GetThreadName();
RAY_LOG(DEBUG) << "First accessed in thread " << thread_name_;
id_ = std::this_thread::get_id();
}

RAY_CHECK(id_ == std::this_thread::get_id())
<< "A variable private to thread " << thread_name_ << " was accessed in thread "
<< GetThreadName();
}

T t_;
mutable std::string thread_name_;
mutable std::thread::id id_;
mutable std::mutex mutex_;
mutable ThreadChecker thread_checker_;
};

class ExponentialBackOff {
Expand Down

0 comments on commit ba41ae9

Please sign in to comment.