Reviewers: Jarin,
Message:
There were multiple deadlocks. I described them briefly in the comments, but
I'll go into more detail here.
1) Originally I didn't lock in NotifyWake. That is mostly OK on POSIX, but
fails
on Windows because the CondVar implementation expects it (there is a DCHECK
in
ConditionVariable::Wait() that can fire -- I believe it is because SetEvent
could cause a thread switch before notified is set to true).
It turns out that the lock is useful anyway (for preventing lost wakeups),
so I
added it in. This causes a lock inversion with the execution lock when
requesting an interrupt. The fix here is to release the execution lock
before
calling NotifyWake.
2) The second deadlock was much more rare (~2 times out of 10000), and
looked
like it was Windows-only. This inversion occurs when the futex lock is held
while calling HandleInterrupts. This can occasionally coincide with the
OptimizingCompilerDispatcher being run on both threads. This object has a
mutex
"output_queue_mutex_" that is acquired in CompileNext and in
InstallOptimizedFunctions. The inversion occurs between this lock and the
futex
lock. The thread requesting the interrupt locks output_queue_mutex_, then
calls
NotifyWake which locks the futex lock. The futexwait thread locks the futex
lock, then tries to lock the output_queue_mutex_ when calling
HandleInterrupts.
Now that I look at it, these are the same deadlock; so I can remove the
unlock/lock pair around the execution lock.
The other thing that was throwing me off was the Time vs. TimeTicks bug,
which
would cause occasional flakes on Windows -- it was hard to tell if those
were
related or not.
https://codereview.chromium.org/1230303005/diff/80001/src/futex-emulation.cc
File src/futex-emulation.cc (right):
https://codereview.chromium.org/1230303005/diff/80001/src/futex-emulation.cc#newcode138
src/futex-emulation.cc:138: if (node->CheckInterruptedAndClear()) {
On 2015/08/11 at 08:21:49, Jarin wrote:
Why can't you read and clear the interrupt flag while still holding
the lock? That way, you would not have to resort to the CASing, no? (You
could also use lexically scoped guards.)
Good point, I'll change this.
Description:
Signal a blocked futex if the isolate is interrupted; don't busy-wait
FutexEmulation::Wait can potentially block forever on a condition variable.
We
want to allow this to be interrupted (for a debugger, or to terminate the
thread, for example).
The previous implementation would periodically wake up the waiter to check
for
interrupts. This CL modifies the StackGuard so it wakes the blocked futex if
the thread should be interrupted.
BUG=chromium:497295
[email protected]
LOG=n
Please review this at https://codereview.chromium.org/1230303005/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+102, -31 lines):
M src/execution.cc
M src/futex-emulation.h
M src/futex-emulation.cc
M test/cctest/test-api.cc
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
