Skip to content

Commit

Permalink
making the feature to prefer thread waiter over fiber waiter as an op…
Browse files Browse the repository at this point in the history
…tion

Summary:
As discovered in T152018412, some of the mutexes (TimedMutex) can be accessed by both fibers and
threads, and the existing feature to let thread waiter to steal the mutex from fiber waiter could
cause a sort of live lock issue where the thread waiters keep spinning for resources protected by
the mutex, preventing the fibers who can produce the resource from making progress. The motivation
behind the feature introduced in D4209155 was to resolve the deadlock that could happen when the
mutex is accessed at the same time from the main context of the fiber. However, this could break the
fairness and should be considered as an option for the use case where the main context is not used.

Reviewed By: dmm-fb

Differential Revision: D45931701

fbshipit-source-id: a292d76ce3cfac51fb6f34a58fc447c287c72d22
  • Loading branch information
Jaesoo Lee authored and facebook-github-bot committed Oct 23, 2023
1 parent c71af83 commit aa30a71
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
2 changes: 1 addition & 1 deletion folly/fibers/TimedMutex-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ TimedMutex::LockResult TimedMutex::lockHelper(WaitFunc&& waitFunc) {
return LockResult::SUCCESS;
}

const auto isOnFiber = onFiber();
const auto isOnFiber = options_.stealing_ && onFiber();

if (!isOnFiber && notifiedFiber_ != nullptr) {
// lock() was called on a thread and while some other fiber was already
Expand Down
12 changes: 11 additions & 1 deletion folly/fibers/TimedMutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,16 @@ namespace fibers {
**/
class TimedMutex {
public:
TimedMutex() noexcept {}
struct Options {
constexpr Options(bool stealing = true) : stealing_(stealing) {}
/**
* Prefer thread waiter and steal from fiber waiters if possible
*/
bool stealing_ = true;
};

TimedMutex(Options options = Options()) noexcept
: options_(std::move(options)) {}

~TimedMutex() {
DCHECK(threadWaiters_.empty());
Expand Down Expand Up @@ -80,6 +89,7 @@ class TimedMutex {

using MutexWaiterList = folly::IntrusiveList<MutexWaiter, &MutexWaiter::hook>;

const Options options_;
folly::SpinLock lock_; //< lock to protect waiter list
bool locked_ = false; //< is this locked by some thread?
MutexWaiterList threadWaiters_; //< list of waiters
Expand Down

0 comments on commit aa30a71

Please sign in to comment.