Commit 4510401d authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[inspector][fuzzer] Fix termination

Joining the thread from the watchdog is problematic, since e.g.
{pthread_join} (the implementation of {Thread::Join} on POSIX systems)
has undefined behaviour if multiple threads try to join at the same
time. In practice, this leads to deadlocks.

Thus implement termination by just calling {TaskRunner::Terminate}, but
not {TaskRunner::Join}. This fixes the deadlocks in the inspector
fuzzer.
The inspector test binary is fixed simarly, even though there it seems
to not cause problems so far.

In both files, the {Terminate} function is inlined into callers because
it's only a single line now, with one to two users.

Also, replace the single fuzzer test (which is invalid javascript) by
two tests: One called "invalid" explicitly, still with invalid
javascript, and one empty file, which is valid input. That one
reproduced the deadlock.

R=szuend@chromium.org

Bug: chromium:1142437
Change-Id: I8fb98b0cdbf3ceff6af6849397e5da5a4e9acd3c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2526384Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71054}
parent c9e883e8
......@@ -32,13 +32,6 @@ namespace {
base::SmallVector<TaskRunner*, 2> task_runners;
void Terminate() {
for (TaskRunner* r : task_runners) {
r->Terminate();
r->Join();
}
}
class UtilsExtension : public IsolateData::SetupGlobalTask {
public:
~UtilsExtension() override = default;
......@@ -88,7 +81,9 @@ class UtilsExtension : public IsolateData::SetupGlobalTask {
static TaskRunner* backend_runner_;
static void Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
Terminate();
// Only terminate, so not join the threads here, since joining concurrently
// from multiple threads can be undefined behaviour (see pthread_join).
for (TaskRunner* task_runner : task_runners) task_runner->Terminate();
}
static void CompileAndRunWithOrigin(
......@@ -561,7 +556,7 @@ class Watchdog final : public base::Thread {
private:
void Run() override {
if (semaphore_->WaitFor(kMaxExecutionSeconds)) return;
Terminate();
for (TaskRunner* task_runner : task_runners) task_runner->Terminate();
}
base::Semaphore* const semaphore_;
......
......@@ -41,13 +41,6 @@ namespace {
base::SmallVector<TaskRunner*, 2> task_runners;
void Terminate() {
for (TaskRunner* task_runner : task_runners) {
task_runner->Terminate();
task_runner->Join();
}
}
class UtilsExtension : public IsolateData::SetupGlobalTask {
public:
~UtilsExtension() override = default;
......@@ -154,7 +147,9 @@ class UtilsExtension : public IsolateData::SetupGlobalTask {
static void Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
fflush(stdout);
fflush(stderr);
Terminate();
// Only terminate, so not join the threads here, since joining concurrently
// from multiple threads can be undefined behaviour (see pthread_join).
for (TaskRunner* task_runner : task_runners) task_runner->Terminate();
}
static void Setlocale(const v8::FunctionCallbackInfo<v8::Value>& args) {
......
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