Commit b488bf43 authored by Istiaque Ahmed's avatar Istiaque Ahmed Committed by Commit Bot

[ESW] Fix IPC handling in WorkerThreadDispatcher after worker shutdown.

IPCs to WorkerThreadDispatcher arrive initially on main thread via
WorkerThreadDispatcher::OnControlMessageReceived() [1]. This IPC is
forwarded to worker thread, via
WorkerThreadDispatcher::OnMessageReceivedOnWorkerThread() [2].

If the worker destruction
(Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread) happens
on worker thread between [1] and [2], IPC forwarding succeeds
(b/c the worker is alive at [1]), but IPC handling doesn't (as
worker data is already cleared while we run [2]).

Fix this by checking worker data's existence while handling the
IPC on worker thread and bailing out.

Bug: 1008143
Test: Locally running ServiceWorkerTest.Update*Extension thousand times.
Change-Id: Id5dd1b4c3d24995228aeb0f1fdd72edc1b77f747
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825169Reviewed-by: default avatarDavid Bertoni <dbertoni@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700087}
parent 07c99c39
......@@ -116,16 +116,9 @@ bool WorkerThreadDispatcher::OnControlMessageReceived(
CHECK(found);
if (worker_thread_id == kMainThreadId)
return false;
base::TaskRunner* runner = GetTaskRunnerFor(worker_thread_id);
// The TaskRunner may have been destroyed, for example, if the extension
// was unloaded, so check before trying to post a task.
if (runner == nullptr)
return false;
bool task_posted = runner->PostTask(
FROM_HERE, base::BindOnce(&WorkerThreadDispatcher::ForwardIPC,
return PostTaskToWorkerThread(
worker_thread_id, base::BindOnce(&WorkerThreadDispatcher::ForwardIPC,
worker_thread_id, message));
DCHECK(task_posted) << "Could not PostTask IPC to worker thread.";
return true;
}
return false;
}
......@@ -150,6 +143,13 @@ void WorkerThreadDispatcher::OnMessageReceivedOnWorkerThread(
int worker_thread_id,
const IPC::Message& message) {
CHECK_EQ(content::WorkerThread::GetCurrentId(), worker_thread_id);
// If the worker state was already destroyed via
// Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread, then
// drop this IPC. See https://crbug.com/1008143 for details.
if (!GetServiceWorkerData())
return;
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(WorkerThreadDispatcher, message)
IPC_MESSAGE_HANDLER(ExtensionMsg_ResponseWorker, OnResponseWorker)
......@@ -164,11 +164,16 @@ void WorkerThreadDispatcher::OnMessageReceivedOnWorkerThread(
CHECK(handled);
}
base::TaskRunner* WorkerThreadDispatcher::GetTaskRunnerFor(
int worker_thread_id) {
bool WorkerThreadDispatcher::PostTaskToWorkerThread(int worker_thread_id,
base::OnceClosure task) {
base::AutoLock lock(task_runner_map_lock_);
auto it = task_runner_map_.find(worker_thread_id);
return it == task_runner_map_.end() ? nullptr : it->second;
if (it == task_runner_map_.end())
return false;
bool task_posted = it->second->PostTask(FROM_HERE, std::move(task));
DCHECK(task_posted) << "Could not PostTask IPC to worker thread.";
return task_posted;
}
bool WorkerThreadDispatcher::Send(IPC::Message* message) {
......
......@@ -94,7 +94,7 @@ class WorkerThreadDispatcher : public content::RenderThreadObserver,
void OnMessageReceivedOnWorkerThread(int worker_thread_id,
const IPC::Message& message);
base::TaskRunner* GetTaskRunnerFor(int worker_thread_id);
bool PostTaskToWorkerThread(int worker_thread_id, base::OnceClosure task);
// IPC handlers.
void OnResponseWorker(int worker_thread_id,
......
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