Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(229)

Unified Diff: src/futex-emulation.cc

Issue 1230303005: Signal a blocked futex if the isolate is interrupted; don't busy-wait (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: remove atomic waiting_ Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/futex-emulation.h ('k') | test/cctest/test-api.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/futex-emulation.cc
diff --git a/src/futex-emulation.cc b/src/futex-emulation.cc
index 5a0ce07f1a545389e8d5431f09ff0adf39acc178..b0e514e8af2a11c8094f2438fa651ed4172fb142 100644
--- a/src/futex-emulation.cc
+++ b/src/futex-emulation.cc
@@ -21,6 +21,23 @@ base::LazyInstance<FutexWaitList>::type FutexEmulation::wait_list_ =
LAZY_INSTANCE_INITIALIZER;
+void FutexWaitListNode::NotifyWake() {
+ // Lock the FutexEmulation mutex before notifying. We know that the mutex
+ // will have been unlocked if we are currently waiting on the condition
+ // variable.
+ //
+ // The mutex may also not be locked if the other thread is currently handling
+ // interrupts, or if FutexEmulation::Wait was just called and the mutex
+ // hasn't been locked yet. In either of those cases, we set the interrupted
+ // flag to true, which will be tested after the mutex is re-locked.
+ base::LockGuard<base::Mutex> lock_guard(FutexEmulation::mutex_.Pointer());
+ if (waiting_) {
+ cond_.NotifyOne();
+ interrupted_ = true;
+ }
+}
+
+
FutexWaitList::FutexWaitList() : head_(nullptr), tail_(nullptr) {}
@@ -58,12 +75,6 @@ void FutexWaitList::RemoveNode(FutexWaitListNode* node) {
Object* FutexEmulation::Wait(Isolate* isolate,
Handle<JSArrayBuffer> array_buffer, size_t addr,
int32_t value, double rel_timeout_ms) {
- // We never want to wait longer than this amount of time; this way we can
- // interrupt this thread even if this is an "infinitely blocking" wait.
- // TODO(binji): come up with a better way of interrupting only when
- // necessary, rather than busy-waiting.
- const base::TimeDelta kMaxWaitTime = base::TimeDelta::FromMilliseconds(50);
-
DCHECK(addr < NumberToSize(isolate, array_buffer->byte_length()));
void* backing_store = array_buffer->backing_store();
@@ -103,41 +114,75 @@ Object* FutexEmulation::Wait(Isolate* isolate,
base::TimeTicks start_time = base::TimeTicks::Now();
base::TimeTicks timeout_time = start_time + rel_timeout;
+ base::TimeTicks current_time = start_time;
wait_list_.Pointer()->AddNode(node);
Object* result;
while (true) {
- base::TimeTicks current_time = base::TimeTicks::Now();
- if (use_timeout && current_time > timeout_time) {
- result = Smi::FromInt(Result::kTimedOut);
- break;
+ bool interrupted = node->interrupted_;
+ node->interrupted_ = false;
+
+ // Unlock the mutex here to prevent deadlock from lock ordering between
+ // mutex_ and mutexes locked by HandleInterrupts.
+ mutex_.Pointer()->Unlock();
+
+ // Because the mutex is unlocked, we have to be careful about not dropping
+ // an interrupt. The notification can happen in three different places:
+ // 1) Before Wait is called: the notification will be dropped, but
+ // interrupted_ will be set to 1. This will be checked below.
+ // 2) After interrupted has been checked here, but before mutex_ is
+ // acquired: interrupted is checked again below, with mutex_ locked.
+ // Because the wakeup signal also acquires mutex_, we know it will not
+ // be able to notify until mutex_ is released below, when waiting on the
+ // condition variable.
+ // 3) After the mutex is released in the call to WaitFor(): this
+ // notification will wake up the condition variable. node->waiting() will
+ // be false, so we'll loop and then check interrupts.
+ if (interrupted) {
+ Object* interrupt_object = isolate->stack_guard()->HandleInterrupts();
+ if (interrupt_object->IsException()) {
+ result = interrupt_object;
+ mutex_.Pointer()->Lock();
+ break;
+ }
}
- base::TimeDelta time_until_timeout = timeout_time - current_time;
- base::TimeDelta time_to_wait =
- (use_timeout && time_until_timeout < kMaxWaitTime) ? time_until_timeout
- : kMaxWaitTime;
+ mutex_.Pointer()->Lock();
- bool wait_for_result = node->cond_.WaitFor(mutex_.Pointer(), time_to_wait);
- USE(wait_for_result);
+ if (node->interrupted_) {
+ // An interrupt occured while the mutex_ was unlocked. Don't wait yet.
+ continue;
+ }
if (!node->waiting_) {
result = Smi::FromInt(Result::kOk);
break;
}
- // Spurious wakeup or timeout. Potentially handle interrupts before
- // continuing to wait.
- Object* interrupt_object = isolate->stack_guard()->HandleInterrupts();
- if (interrupt_object->IsException()) {
- result = interrupt_object;
- break;
+ // No interrupts, now wait.
+ if (use_timeout) {
+ current_time = base::TimeTicks::Now();
+ if (current_time >= timeout_time) {
+ result = Smi::FromInt(Result::kTimedOut);
+ break;
+ }
+
+ base::TimeDelta time_until_timeout = timeout_time - current_time;
+ DCHECK(time_until_timeout.InMicroseconds() >= 0);
+ bool wait_for_result =
+ node->cond_.WaitFor(mutex_.Pointer(), time_until_timeout);
+ USE(wait_for_result);
+ } else {
+ node->cond_.Wait(mutex_.Pointer());
}
+
+ // Spurious wakeup, interrupt or timeout.
}
wait_list_.Pointer()->RemoveNode(node);
+ node->waiting_ = false;
return result;
}
« no previous file with comments | « src/futex-emulation.h ('k') | test/cctest/test-api.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698