|
@@ -0,0 +1,285 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Dave Tapuska <[email protected]>
|
|
|
+Date: Thu, 3 Nov 2022 23:16:16 +0000
|
|
|
+Subject: Fix PauseOrFreezeOnWorkerThread with nested Worklets.
|
|
|
+
|
|
|
+Worklets can use the same backing thread which means we can have
|
|
|
+nested WorkerThreads paused. If a parent WorkerThread gets deallocated
|
|
|
+make sure we don't access anything after it is deallocated once the
|
|
|
+nested event queue is released.
|
|
|
+
|
|
|
+BUG=1372695
|
|
|
+
|
|
|
+(cherry picked from commit ff5696ba4bc0f8782e3de40e04685507d9f17fd2)
|
|
|
+
|
|
|
+Change-Id: I176b8f750da5a41d19d1b3a623944d9a2ed4a441
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3953152
|
|
|
+Commit-Queue: Dave Tapuska <[email protected]>
|
|
|
+Reviewed-by: Daniel Cheng <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/heads/main@{#1059485}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004562
|
|
|
+Cr-Commit-Position: refs/branch-heads/5249@{#906}
|
|
|
+Cr-Branched-From: 4f7bea5de862aaa52e6bde5920755a9ef9db120b-refs/heads/main@{#1036826}
|
|
|
+
|
|
|
+diff --git a/third_party/blink/renderer/core/workers/threaded_worklet_test.cc b/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
|
|
|
+index c54d0a91447717f014f44d36e004d9092e98f8bb..dbe40e69a276983bc2df5dcddd7b9b483d2534b1 100644
|
|
|
+--- a/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
|
|
|
++++ b/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
|
|
|
+@@ -234,6 +234,7 @@ class ThreadedWorkletMessagingProxyForTest
|
|
|
+
|
|
|
+ private:
|
|
|
+ friend class ThreadedWorkletTest;
|
|
|
++ FRIEND_TEST_ALL_PREFIXES(ThreadedWorkletTest, NestedRunLoopTermination);
|
|
|
+
|
|
|
+ std::unique_ptr<WorkerThread> CreateWorkerThread() final {
|
|
|
+ return std::make_unique<ThreadedWorkletThreadForTest>(WorkletObjectProxy());
|
|
|
+@@ -280,6 +281,16 @@ class ThreadedWorkletTest : public testing::Test {
|
|
|
+ }
|
|
|
+ Document& GetDocument() { return page_->GetDocument(); }
|
|
|
+
|
|
|
++ void WaitForReady(WorkerThread* worker_thread) {
|
|
|
++ base::WaitableEvent child_waitable;
|
|
|
++ PostCrossThreadTask(
|
|
|
++ *worker_thread->GetTaskRunner(TaskType::kInternalTest), FROM_HERE,
|
|
|
++ CrossThreadBindOnce(&base::WaitableEvent::Signal,
|
|
|
++ CrossThreadUnretained(&child_waitable)));
|
|
|
++
|
|
|
++ child_waitable.Wait();
|
|
|
++ }
|
|
|
++
|
|
|
+ private:
|
|
|
+ std::unique_ptr<DummyPageHolder> page_;
|
|
|
+ Persistent<ThreadedWorkletMessagingProxyForTest> messaging_proxy_;
|
|
|
+@@ -403,4 +414,40 @@ TEST_F(ThreadedWorkletTest, TaskRunner) {
|
|
|
+ test::EnterRunLoop();
|
|
|
+ }
|
|
|
+
|
|
|
++TEST_F(ThreadedWorkletTest, NestedRunLoopTermination) {
|
|
|
++ MessagingProxy()->Start();
|
|
|
++
|
|
|
++ ThreadedWorkletMessagingProxyForTest* second_messaging_proxy =
|
|
|
++ MakeGarbageCollected<ThreadedWorkletMessagingProxyForTest>(
|
|
|
++ GetExecutionContext());
|
|
|
++
|
|
|
++ // Get a nested event loop where the first one is on the stack
|
|
|
++ // and the second is still alive.
|
|
|
++ second_messaging_proxy->Start();
|
|
|
++
|
|
|
++ // Wait until the workers are setup and ready to accept work before we
|
|
|
++ // pause them.
|
|
|
++ WaitForReady(GetWorkerThread());
|
|
|
++ WaitForReady(second_messaging_proxy->GetWorkerThread());
|
|
|
++
|
|
|
++ // Pause the second worker, then the first.
|
|
|
++ second_messaging_proxy->GetWorkerThread()->Pause();
|
|
|
++ GetWorkerThread()->Pause();
|
|
|
++
|
|
|
++ // Resume then terminate the second worker.
|
|
|
++ second_messaging_proxy->GetWorkerThread()->Resume();
|
|
|
++ second_messaging_proxy->GetWorkerThread()->Terminate();
|
|
|
++ second_messaging_proxy = nullptr;
|
|
|
++
|
|
|
++ // Now resume the first worker.
|
|
|
++ GetWorkerThread()->Resume();
|
|
|
++
|
|
|
++ // Make sure execution still works without crashing.
|
|
|
++ PostCrossThreadTask(
|
|
|
++ *GetWorkerThread()->GetTaskRunner(TaskType::kInternalTest), FROM_HERE,
|
|
|
++ CrossThreadBindOnce(&ThreadedWorkletThreadForTest::TestTaskRunner,
|
|
|
++ CrossThreadUnretained(GetWorkerThread())));
|
|
|
++ test::EnterRunLoop();
|
|
|
++}
|
|
|
++
|
|
|
+ } // namespace blink
|
|
|
+diff --git a/third_party/blink/renderer/core/workers/worker_thread.cc b/third_party/blink/renderer/core/workers/worker_thread.cc
|
|
|
+index 892b3e58443f1a82a6a41c6d52df7d2d701b377d..3a98c2d192e1e47d1586d1dab1fc9e9dc9daf83b 100644
|
|
|
+--- a/third_party/blink/renderer/core/workers/worker_thread.cc
|
|
|
++++ b/third_party/blink/renderer/core/workers/worker_thread.cc
|
|
|
+@@ -30,7 +30,6 @@
|
|
|
+ #include <memory>
|
|
|
+ #include <utility>
|
|
|
+
|
|
|
+-#include "base/auto_reset.h"
|
|
|
+ #include "base/metrics/histogram_functions.h"
|
|
|
+ #include "base/synchronization/waitable_event.h"
|
|
|
+ #include "base/threading/thread_restrictions.h"
|
|
|
+@@ -593,6 +592,7 @@ void WorkerThread::InitializeOnWorkerThread(
|
|
|
+ const absl::optional<WorkerBackingThreadStartupData>& thread_startup_data,
|
|
|
+ std::unique_ptr<WorkerDevToolsParams> devtools_params) {
|
|
|
+ DCHECK(IsCurrentThread());
|
|
|
++ backing_thread_weak_factory_.emplace(this);
|
|
|
+ worker_reporting_proxy_.WillInitializeWorkerContext();
|
|
|
+ {
|
|
|
+ TRACE_EVENT0("blink.worker", "WorkerThread::InitializeWorkerContext");
|
|
|
+@@ -728,11 +728,13 @@ void WorkerThread::PrepareForShutdownOnWorkerThread() {
|
|
|
+ SetThreadState(ThreadState::kReadyToShutdown);
|
|
|
+ }
|
|
|
+
|
|
|
++ backing_thread_weak_factory_ = absl::nullopt;
|
|
|
+ if (pause_or_freeze_count_ > 0) {
|
|
|
+ DCHECK(nested_runner_);
|
|
|
+ pause_or_freeze_count_ = 0;
|
|
|
+ nested_runner_->QuitNow();
|
|
|
+ }
|
|
|
++ pause_handle_.reset();
|
|
|
+
|
|
|
+ {
|
|
|
+ v8::HandleScope handle_scope(GetIsolate());
|
|
|
+@@ -883,8 +885,7 @@ void WorkerThread::PauseOrFreezeOnWorkerThread(
|
|
|
+ if (pause_or_freeze_count_ > 1)
|
|
|
+ return;
|
|
|
+
|
|
|
+- std::unique_ptr<scheduler::WorkerScheduler::PauseHandle> pause_handle =
|
|
|
+- GetScheduler()->Pause();
|
|
|
++ pause_handle_ = GetScheduler()->Pause();
|
|
|
+ {
|
|
|
+ // Since the nested message loop runner needs to be created and destroyed on
|
|
|
+ // the same thread we allocate and destroy a new message loop runner each
|
|
|
+@@ -892,13 +893,20 @@ void WorkerThread::PauseOrFreezeOnWorkerThread(
|
|
|
+ // the worker thread such that the resume/terminate can quit this runner.
|
|
|
+ std::unique_ptr<Platform::NestedMessageLoopRunner> nested_runner =
|
|
|
+ Platform::Current()->CreateNestedMessageLoopRunner();
|
|
|
+- base::AutoReset<Platform::NestedMessageLoopRunner*> nested_runner_autoreset(
|
|
|
+- &nested_runner_, nested_runner.get());
|
|
|
++ auto weak_this = backing_thread_weak_factory_->GetWeakPtr();
|
|
|
++ nested_runner_ = nested_runner.get();
|
|
|
+ nested_runner->Run();
|
|
|
++
|
|
|
++ // Careful `this` may be destroyed.
|
|
|
++ if (!weak_this) {
|
|
|
++ return;
|
|
|
++ }
|
|
|
++ nested_runner_ = nullptr;
|
|
|
+ }
|
|
|
+ GlobalScope()->SetDefersLoadingForResourceFetchers(LoaderFreezeMode::kNone);
|
|
|
+ GlobalScope()->SetIsInBackForwardCache(false);
|
|
|
+ GlobalScope()->SetLifecycleState(mojom::blink::FrameLifecycleState::kRunning);
|
|
|
++ pause_handle_.reset();
|
|
|
+ }
|
|
|
+
|
|
|
+ void WorkerThread::ResumeOnWorkerThread() {
|
|
|
+diff --git a/third_party/blink/renderer/core/workers/worker_thread.h b/third_party/blink/renderer/core/workers/worker_thread.h
|
|
|
+index 834aabc12fe0c43f69bb3a5fa669571c84847c6d..2431c406268a3631df157a24c3fd7accf184cccc 100644
|
|
|
+--- a/third_party/blink/renderer/core/workers/worker_thread.h
|
|
|
++++ b/third_party/blink/renderer/core/workers/worker_thread.h
|
|
|
+@@ -31,6 +31,7 @@
|
|
|
+
|
|
|
+ #include "base/gtest_prod_util.h"
|
|
|
+ #include "base/memory/scoped_refptr.h"
|
|
|
++#include "base/memory/weak_ptr.h"
|
|
|
+ #include "base/synchronization/waitable_event.h"
|
|
|
+ #include "base/task/single_thread_task_runner.h"
|
|
|
+ #include "base/thread_annotations.h"
|
|
|
+@@ -80,7 +81,7 @@ struct WorkerMainScriptLoadParameters;
|
|
|
+ // abstract class. Multiple WorkerThreads may share one WorkerBackingThread for
|
|
|
+ // worklets.
|
|
|
+ //
|
|
|
+-// WorkerThread start and termination must be initiated on the main thread and
|
|
|
++// WorkerThread start and termination must be initiated on the parent thread and
|
|
|
+ // an actual task is executed on the worker thread.
|
|
|
+ //
|
|
|
+ // When termination starts, (debugger) tasks on WorkerThread are handled as
|
|
|
+@@ -103,7 +104,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ ~WorkerThread() override;
|
|
|
+
|
|
|
+ // Starts the underlying thread and creates the global scope. Called on the
|
|
|
+- // main thread.
|
|
|
++ // parent thread.
|
|
|
+ // Startup data for WorkerBackingThread is absl::nullopt if |this| doesn't own
|
|
|
+ // the underlying WorkerBackingThread.
|
|
|
+ // TODO(nhiroki): We could separate WorkerBackingThread initialization from
|
|
|
+@@ -115,14 +116,14 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ std::unique_ptr<WorkerDevToolsParams>);
|
|
|
+
|
|
|
+ // Posts a task to evaluate a top-level classic script on the worker thread.
|
|
|
+- // Called on the main thread after Start().
|
|
|
++ // Called on the parent thread after Start().
|
|
|
+ void EvaluateClassicScript(const KURL& script_url,
|
|
|
+ const String& source_code,
|
|
|
+ std::unique_ptr<Vector<uint8_t>> cached_meta_data,
|
|
|
+ const v8_inspector::V8StackTraceId& stack_id);
|
|
|
+
|
|
|
+ // Posts a task to fetch and run a top-level classic script on the worker
|
|
|
+- // thread. Called on the main thread after Start().
|
|
|
++ // thread. Called on the parent thread after Start().
|
|
|
+ void FetchAndRunClassicScript(
|
|
|
+ const KURL& script_url,
|
|
|
+ std::unique_ptr<WorkerMainScriptLoadParameters>
|
|
|
+@@ -133,7 +134,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ const v8_inspector::V8StackTraceId& stack_id);
|
|
|
+
|
|
|
+ // Posts a task to fetch and run a top-level module script on the worker
|
|
|
+- // thread. Called on the main thread after Start().
|
|
|
++ // thread. Called on the parent thread after Start().
|
|
|
+ void FetchAndRunModuleScript(
|
|
|
+ const KURL& script_url,
|
|
|
+ std::unique_ptr<WorkerMainScriptLoadParameters>
|
|
|
+@@ -154,7 +155,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ // Terminates the worker thread. Subclasses of WorkerThread can override this
|
|
|
+ // to do cleanup. The default behavior is to call Terminate() and
|
|
|
+ // synchronously call EnsureScriptExecutionTerminates() to ensure the thread
|
|
|
+- // is quickly terminated. Called on the main thread.
|
|
|
++ // is quickly terminated. Called on the parent thread.
|
|
|
+ virtual void TerminateForTesting();
|
|
|
+
|
|
|
+ // Thread::TaskObserver.
|
|
|
+@@ -181,7 +182,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ void DebuggerTaskStarted();
|
|
|
+ void DebuggerTaskFinished();
|
|
|
+
|
|
|
+- // Callable on both the main thread and the worker thread.
|
|
|
++ // Callable on both the parent thread and the worker thread.
|
|
|
+ const base::UnguessableToken& GetDevToolsWorkerToken() const {
|
|
|
+ return devtools_worker_token_;
|
|
|
+ }
|
|
|
+@@ -325,7 +326,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ // already shutting down. Does not terminate if a debugger task is running,
|
|
|
+ // because the debugger task is guaranteed to finish and it heavily uses V8
|
|
|
+ // API calls which would crash after forcible script termination. Called on
|
|
|
+- // the main thread.
|
|
|
++ // the parent thread.
|
|
|
+ void EnsureScriptExecutionTerminates(ExitCode) LOCKS_EXCLUDED(mutex_);
|
|
|
+
|
|
|
+ // These are called in this order during worker thread startup.
|
|
|
+@@ -410,7 +411,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ // A unique identifier among all WorkerThreads.
|
|
|
+ const int worker_thread_id_;
|
|
|
+
|
|
|
+- // Set on the main thread.
|
|
|
++ // Set on the parent thread.
|
|
|
+ bool requested_to_terminate_ GUARDED_BY(mutex_) = false;
|
|
|
+
|
|
|
+ ThreadState thread_state_ GUARDED_BY(mutex_) = ThreadState::kNotStarted;
|
|
|
+@@ -444,7 +445,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ TaskTypeTraits>;
|
|
|
+ TaskRunnerHashMap worker_task_runners_;
|
|
|
+
|
|
|
+- // This lock protects shared states between the main thread and the worker
|
|
|
++ // This lock protects shared states between the parent thread and the worker
|
|
|
+ // thread. See thread-safety annotations (e.g., GUARDED_BY) in this header
|
|
|
+ // file.
|
|
|
+ Mutex mutex_;
|
|
|
+@@ -453,6 +454,10 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ // only on the worker thread.
|
|
|
+ int pause_or_freeze_count_ = 0;
|
|
|
+
|
|
|
++ // The `PauseHandle` needs to be destroyed before the scheduler is destroyed
|
|
|
++ // otherwise we will hit a DCHECK.
|
|
|
++ std::unique_ptr<scheduler::WorkerScheduler::PauseHandle> pause_handle_;
|
|
|
++
|
|
|
+ // A nested message loop for handling pausing. Pointer is not owned. Used only
|
|
|
+ // on the worker thread.
|
|
|
+ Platform::NestedMessageLoopRunner* nested_runner_ = nullptr;
|
|
|
+@@ -479,6 +484,12 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
|
|
|
+ HashSet<std::unique_ptr<InterruptData>> pending_interrupts_
|
|
|
+ GUARDED_BY(mutex_);
|
|
|
+
|
|
|
++ // Since the WorkerThread is allocated and deallocated on the parent thread,
|
|
|
++ // we need a WeakPtrFactory that is allocated and cleared on the backing
|
|
|
++ // thread.
|
|
|
++ absl::optional<base::WeakPtrFactory<WorkerThread>>
|
|
|
++ backing_thread_weak_factory_;
|
|
|
++
|
|
|
+ THREAD_CHECKER(parent_thread_checker_);
|
|
|
+ };
|
|
|
+
|