Commit e980d0e8 authored by Anna Henningsen's avatar Anna Henningsen Committed by Commit Bot

[SAB] Fix flake in Atomics.wait, part II

Refs: https://ci.chromium.org/buildbot/client.v8/V8%20Linux64%20TSAN/21047

Bug: v8:7836
Change-Id: Ic825065de419bbab97972d9b7a2beb6527b0a48a
Reviewed-on: https://chromium-review.googlesource.com/1103560
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: 's avatarBen Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53810}
parent 71892ad9
...@@ -74,7 +74,14 @@ void FutexWaitList::RemoveNode(FutexWaitListNode* node) { ...@@ -74,7 +74,14 @@ void FutexWaitList::RemoveNode(FutexWaitListNode* node) {
} }
void AtomicsWaitWakeHandle::Wake() { void AtomicsWaitWakeHandle::Wake() {
stopped_ = true; // Adding a separate `NotifyWake()` variant that doesn't acquire the lock
// itself would likely just add unnecessary complexity..
// The split lock by itself isn’t an issue, as long as the caller properly
// synchronizes this with the closing `AtomicsWaitCallback`.
{
base::LockGuard<base::Mutex> lock_guard(FutexEmulation::mutex_.Pointer());
stopped_ = true;
}
isolate_->futex_wait_list_node()->NotifyWake(); isolate_->futex_wait_list_node()->NotifyWake();
} }
...@@ -126,6 +133,9 @@ Object* FutexEmulation::Wait(Isolate* isolate, ...@@ -126,6 +133,9 @@ Object* FutexEmulation::Wait(Isolate* isolate,
do { // Not really a loop, just makes it easier to break out early. do { // Not really a loop, just makes it easier to break out early.
base::LockGuard<base::Mutex> lock_guard(mutex_.Pointer()); base::LockGuard<base::Mutex> lock_guard(mutex_.Pointer());
// Reset node->waiting_ = false when leaving this scope (but while
// still holding the lock).
ResetWaitingOnScopeExit reset_waiting(node);
if (*p != value) { if (*p != value) {
result = isolate->heap()->not_equal(); result = isolate->heap()->not_equal();
...@@ -214,8 +224,6 @@ Object* FutexEmulation::Wait(Isolate* isolate, ...@@ -214,8 +224,6 @@ Object* FutexEmulation::Wait(Isolate* isolate,
wait_list_.Pointer()->RemoveNode(node); wait_list_.Pointer()->RemoveNode(node);
} while (0); } while (0);
node->waiting_ = false;
isolate->RunAtomicsWaitCallback(callback_result, array_buffer, addr, value, isolate->RunAtomicsWaitCallback(callback_result, array_buffer, addr, value,
rel_timeout_ms, nullptr); rel_timeout_ms, nullptr);
......
...@@ -62,6 +62,7 @@ class FutexWaitListNode { ...@@ -62,6 +62,7 @@ class FutexWaitListNode {
private: private:
friend class FutexEmulation; friend class FutexEmulation;
friend class FutexWaitList; friend class FutexWaitList;
friend class ResetWaitingOnScopeExit;
base::ConditionVariable cond_; base::ConditionVariable cond_;
// prev_ and next_ are protected by FutexEmulation::mutex_. // prev_ and next_ are protected by FutexEmulation::mutex_.
...@@ -70,7 +71,8 @@ class FutexWaitListNode { ...@@ -70,7 +71,8 @@ class FutexWaitListNode {
void* backing_store_; void* backing_store_;
size_t wait_addr_; size_t wait_addr_;
// waiting_ and interrupted_ are protected by FutexEmulation::mutex_ // waiting_ and interrupted_ are protected by FutexEmulation::mutex_
// if this node is currently contained in FutexEmulation::wait_list_. // if this node is currently contained in FutexEmulation::wait_list_
// or an AtomicsWaitWakeHandle has access to it.
bool waiting_; bool waiting_;
bool interrupted_; bool interrupted_;
...@@ -94,6 +96,16 @@ class FutexWaitList { ...@@ -94,6 +96,16 @@ class FutexWaitList {
DISALLOW_COPY_AND_ASSIGN(FutexWaitList); DISALLOW_COPY_AND_ASSIGN(FutexWaitList);
}; };
class ResetWaitingOnScopeExit {
public:
explicit ResetWaitingOnScopeExit(FutexWaitListNode* node) : node_(node) {}
~ResetWaitingOnScopeExit() { node_->waiting_ = false; }
private:
FutexWaitListNode* node_;
DISALLOW_COPY_AND_ASSIGN(ResetWaitingOnScopeExit);
};
class FutexEmulation : public AllStatic { class FutexEmulation : public AllStatic {
public: public:
...@@ -124,6 +136,7 @@ class FutexEmulation : public AllStatic { ...@@ -124,6 +136,7 @@ class FutexEmulation : public AllStatic {
private: private:
friend class FutexWaitListNode; friend class FutexWaitListNode;
friend class AtomicsWaitWakeHandle;
// `mutex_` protects the composition of `wait_list_` (i.e. no elements may be // `mutex_` protects the composition of `wait_list_` (i.e. no elements may be
// added or removed without holding this mutex), as well as the `waiting_` // added or removed without holding this mutex), as well as the `waiting_`
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment