Commit 23af711f authored by Eric Karl's avatar Eric Karl Committed by Commit Bot

OOP-D: Fix deadlock during Viz thread shutdown

Currently, VizMainImpl::ExitProcess has a workaround to avoid deadlock
when shutting down the gpu process. Unfortunately, this is only wired up
to one of the ways to shut down. Other shutdown calls originating from
ChildProcessImpl miss this logic and hit the same deadlock.

This change passes a helper to ChildProcessImpl, allowing it to correctly
handle shutdown. This helper posts a task to the GPU main thread before
starting shutdown to ensure both that we're on the right thread as well
as that we're post-init.

Bug: 901396
Change-Id: I58cff914def5c05f7b9e6b6f58cb319153d685dd
Reviewed-on: https://chromium-review.googlesource.com/c/1315538Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605043}
parent af755396
...@@ -109,15 +109,15 @@ class VizMainImpl : public gpu::GpuSandboxHelper, public mojom::VizMain { ...@@ -109,15 +109,15 @@ class VizMainImpl : public gpu::GpuSandboxHelper, public mojom::VizMain {
return discardable_shared_memory_manager_.get(); return discardable_shared_memory_manager_.get();
} }
// Cleanly exits the process.
void ExitProcess();
private: private:
// Initializes GPU's UkmRecorder if GPU is running in it's own process. // Initializes GPU's UkmRecorder if GPU is running in it's own process.
void CreateUkmRecorderIfNeeded(service_manager::Connector* connector); void CreateUkmRecorderIfNeeded(service_manager::Connector* connector);
void CreateFrameSinkManagerInternal(mojom::FrameSinkManagerParamsPtr params); void CreateFrameSinkManagerInternal(mojom::FrameSinkManagerParamsPtr params);
// Cleanly exits the process.
void ExitProcess();
// gpu::GpuSandboxHelper: // gpu::GpuSandboxHelper:
void PreSandboxStartup() override; void PreSandboxStartup() override;
bool EnsureSandboxInitialized(gpu::GpuWatchdogThread* watchdog_thread, bool EnsureSandboxInitialized(gpu::GpuWatchdogThread* watchdog_thread,
......
...@@ -178,10 +178,11 @@ GpuChildThread::GpuChildThread(const InProcessChildThreadParams& params, ...@@ -178,10 +178,11 @@ GpuChildThread::GpuChildThread(const InProcessChildThreadParams& params,
GpuChildThread::GpuChildThread(base::RepeatingClosure quit_closure, GpuChildThread::GpuChildThread(base::RepeatingClosure quit_closure,
const ChildThreadImpl::Options& options, const ChildThreadImpl::Options& options,
std::unique_ptr<gpu::GpuInit> gpu_init) std::unique_ptr<gpu::GpuInit> gpu_init)
: ChildThreadImpl(std::move(quit_closure), options), : ChildThreadImpl(MakeQuitSafelyClosure(), options),
viz_main_(this, viz_main_(this,
CreateVizMainDependencies(GetConnector()), CreateVizMainDependencies(GetConnector()),
std::move(gpu_init)), std::move(gpu_init)),
quit_closure_(std::move(quit_closure)),
weak_factory_(this) { weak_factory_(this) {
if (in_process_gpu()) { if (in_process_gpu()) {
DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
...@@ -294,7 +295,7 @@ void GpuChildThread::PostCompositorThreadCreated( ...@@ -294,7 +295,7 @@ void GpuChildThread::PostCompositorThreadCreated(
} }
void GpuChildThread::QuitMainMessageLoop() { void GpuChildThread::QuitMainMessageLoop() {
ProcessShutdown(); quit_closure_.Run();
} }
void GpuChildThread::BindServiceFactoryRequest( void GpuChildThread::BindServiceFactoryRequest(
...@@ -316,6 +317,28 @@ void GpuChildThread::OnMemoryPressure( ...@@ -316,6 +317,28 @@ void GpuChildThread::OnMemoryPressure(
SkGraphics::PurgeAllCaches(); SkGraphics::PurgeAllCaches();
} }
void GpuChildThread::QuitSafelyHelper(
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
// Post a new task (even if we're called on the |task_runner|'s thread) to
// ensure that we are post-init.
task_runner->PostTask(
FROM_HERE, base::BindOnce([]() {
ChildThreadImpl* current_child_thread = ChildThreadImpl::current();
if (!current_child_thread)
return;
GpuChildThread* gpu_child_thread =
static_cast<GpuChildThread*>(current_child_thread);
gpu_child_thread->viz_main_.ExitProcess();
}));
}
// Returns a closure which calls into the VizMainImpl to perform shutdown
// before quitting the main message loop. Must be called on the main thread.
base::RepeatingClosure GpuChildThread::MakeQuitSafelyClosure() {
return base::BindRepeating(&GpuChildThread::QuitSafelyHelper,
base::ThreadTaskRunnerHandle::Get());
}
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// static // static
std::unique_ptr<media::AndroidOverlay> GpuChildThread::CreateAndroidOverlay( std::unique_ptr<media::AndroidOverlay> GpuChildThread::CreateAndroidOverlay(
......
...@@ -89,6 +89,12 @@ class GpuChildThread : public ChildThreadImpl, ...@@ -89,6 +89,12 @@ class GpuChildThread : public ChildThreadImpl,
void BindServiceFactoryRequest( void BindServiceFactoryRequest(
service_manager::mojom::ServiceFactoryRequest request); service_manager::mojom::ServiceFactoryRequest request);
// Returns a closure which calls into the VizMainImpl to perform shutdown
// before quitting the main message loop. Must be called on the main thread.
static base::RepeatingClosure MakeQuitSafelyClosure();
static void QuitSafelyHelper(
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
static std::unique_ptr<media::AndroidOverlay> CreateAndroidOverlay( static std::unique_ptr<media::AndroidOverlay> CreateAndroidOverlay(
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner, scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
...@@ -110,6 +116,9 @@ class GpuChildThread : public ChildThreadImpl, ...@@ -110,6 +116,9 @@ class GpuChildThread : public ChildThreadImpl,
// Holds a closure that releases pending interface requests on the IO thread. // Holds a closure that releases pending interface requests on the IO thread.
base::Closure release_pending_requests_closure_; base::Closure release_pending_requests_closure_;
// A closure which quits the main message loop.
base::RepeatingClosure quit_closure_;
std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_; std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;
base::WeakPtrFactory<GpuChildThread> weak_factory_; base::WeakPtrFactory<GpuChildThread> weak_factory_;
......
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