diff options
author | Michal Klocek <michal.klocek@qt.io> | 2020-02-27 13:09:46 +0100 |
---|---|---|
committer | Michal Klocek <michal.klocek@qt.io> | 2020-03-02 14:19:17 +0100 |
commit | 0fe9c7803463f12c6ceea74dec42fb66c27b838f (patch) | |
tree | 9e803c0316d705d6ce4bb2e3487d70c13ee5a39d /src | |
parent | df5d831bae99662fab43ed2628187113c18aac2c (diff) |
Fix deadlocks on WebEngineContext destruction
This patch aims to handle 3 deadlocks on exit:
(1) Do not attempt to restart Viz thread on shutdown,
this will make deadlock with no separate gpu thread,
since force shut down of FrameSinkManager requires
interaction with gpu process.
(2) QCoreApplication, QGuiApplication, QApplication
behave differently when doing qt post routine,
~QGuiApplication calls the post routine handler after
closing event dispatcher, which will on windows stop processing
timer events, which we need to pump message loop when
shuting down viz. Do not use QEventLoop and switch
to active pulling. The proper solution is to fix QGuiApplication
destructor to call post routine first, but this change might
have side effects on already existing user code.
(3) Since 7f1649b438329e we delete root frame sink asynchronously,
which will in gpu thread running in separate thread create a deadlock.
Viz requires gpu to destruct root frame sink, however if main process
tries to close gpu process this will in turn try close viz, but viz calls
back gpu now since the root frame sink is not destroyed.
Use the same solution as in (1).
Change-Id: Ic6bc904bdac90ee01a5c5b9398a2e2746be3bbd8
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/core/web_engine_context.cpp | 51 | ||||
-rw-r--r-- | src/core/web_engine_context.h | 3 | ||||
-rw-r--r-- | src/core/web_engine_context_threads.cpp | 18 |
3 files changed, 42 insertions, 30 deletions
diff --git a/src/core/web_engine_context.cpp b/src/core/web_engine_context.cpp index c34d3c612..286842a20 100644 --- a/src/core/web_engine_context.cpp +++ b/src/core/web_engine_context.cpp @@ -49,6 +49,9 @@ #include "base/task/sequence_manager/thread_controller_with_message_pump_impl.h" #include "base/threading/thread_restrictions.h" #include "cc/base/switches.h" +#include "content/gpu/gpu_child_thread.h" +#include "content/browser/compositor/surface_utils.h" +#include "components/viz/host/host_frame_sink_manager.h" #if QT_CONFIG(webengine_printing_and_pdf) #include "chrome/browser/printing/print_job_manager.h" #include "components/printing/browser/features.h" @@ -211,6 +214,26 @@ bool usingSoftwareDynamicGL() #endif } +static bool waitForViz = false; +static void completeVizCleanup() +{ + waitForViz = false; +} + +static void cleanupVizProcess() +{ + auto gpuChildThread = content::GpuChildThread::instance(); + if (!gpuChildThread) + return; + auto vizMain = gpuChildThread->viz_main(); + auto vizCompositorThreadRunner = vizMain->viz_compositor_thread_runner(); + if (!vizCompositorThreadRunner) + return; + waitForViz = true; + content::GetHostFrameSinkManager()->SetConnectionLostCallback(base::DoNothing()); + vizCompositorThreadRunner->CleanupForShutdown(base::BindOnce(&completeVizCleanup)); +} + scoped_refptr<QtWebEngineCore::WebEngineContext> WebEngineContext::m_handle; bool WebEngineContext::m_destroyed = false; @@ -257,14 +280,21 @@ void WebEngineContext::destroy() if (m_devtoolsServer) m_devtoolsServer->stop(); - // Normally the GPU thread is shut down when the GpuProcessHost is destroyed - // on IO thread (triggered by ~BrowserMainRunner). But by that time the UI - // task runner is not working anymore so we need to do this earlier. - destroyGpuProcess(m_threadedGpu); base::MessagePump::Delegate *delegate = static_cast<base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl *>( m_runLoop->delegate_); + // Normally the GPU thread is shut down when the GpuProcessHost is destroyed + // on IO thread (triggered by ~BrowserMainRunner). But by that time the UI + // task runner is not working anymore so we need to do this earlier. + if (features::IsVizDisplayCompositorEnabled()) { + cleanupVizProcess(); + while (waitForViz) { + while (delegate->DoWork()){} + QThread::msleep(50); + } + } + destroyGpuProcess(); // Flush the UI message loop before quitting. while (delegate->DoWork()) { } @@ -412,8 +442,7 @@ static void appendToFeatureSwitch(base::CommandLine *commandLine, const char *fe } WebEngineContext::WebEngineContext() - : m_threadedGpu(true) - , m_mainDelegate(new ContentMainDelegateQt) + : m_mainDelegate(new ContentMainDelegateQt) , m_globalQObject(new QObject()) { #if defined(Q_OS_MACOS) @@ -515,13 +544,13 @@ WebEngineContext::WebEngineContext() if (isDesktopGLOrSoftware || isGLES2Context) parsedCommandLine->AppendSwitch(switches::kDisableES3GLContext); #endif - + bool threadedGpu = false; #ifndef QT_NO_OPENGL - m_threadedGpu = QOpenGLContext::supportsThreadedOpenGL(); + threadedGpu = QOpenGLContext::supportsThreadedOpenGL(); #endif - m_threadedGpu = m_threadedGpu && !qEnvironmentVariableIsSet(kDisableInProcGpuThread); + threadedGpu = threadedGpu && !qEnvironmentVariableIsSet(kDisableInProcGpuThread); - bool enableViz = ((m_threadedGpu && !parsedCommandLine->HasSwitch("disable-viz-display-compositor")) + bool enableViz = ((threadedGpu && !parsedCommandLine->HasSwitch("disable-viz-display-compositor")) || parsedCommandLine->HasSwitch("enable-viz-display-compositor")); parsedCommandLine->RemoveSwitch("disable-viz-display-compositor"); parsedCommandLine->RemoveSwitch("enable-viz-display-compositor"); @@ -660,7 +689,7 @@ WebEngineContext::WebEngineContext() parsedCommandLine->AppendSwitch(switches::kDisableGpu); } - registerMainThreadFactories(m_threadedGpu); + registerMainThreadFactories(threadedGpu); SetContentClient(new ContentClientQt); diff --git a/src/core/web_engine_context.h b/src/core/web_engine_context.h index edced9b42..ac0536596 100644 --- a/src/core/web_engine_context.h +++ b/src/core/web_engine_context.h @@ -129,9 +129,8 @@ private: ~WebEngineContext(); static void registerMainThreadFactories(bool threaded); - static void destroyGpuProcess(bool threaded); + static void destroyGpuProcess(); - bool m_threadedGpu; std::unique_ptr<base::RunLoop> m_runLoop; std::unique_ptr<ContentMainDelegateQt> m_mainDelegate; std::unique_ptr<content::ContentMainRunner> m_contentRunner; diff --git a/src/core/web_engine_context_threads.cpp b/src/core/web_engine_context_threads.cpp index f0f055676..c7440dd3f 100644 --- a/src/core/web_engine_context_threads.cpp +++ b/src/core/web_engine_context_threads.cpp @@ -92,20 +92,6 @@ struct GpuThreadControllerQt : content::GpuThreadController s_gpuProcess->set_main_thread(childThread); } - static void cleanupVizProcess() - { - auto gpuChildThread = content::GpuChildThread::instance(); - if (!gpuChildThread) - return; - auto vizMain = gpuChildThread->viz_main(); - auto vizCompositorThreadRunner = vizMain->viz_compositor_thread_runner(); - if (!vizCompositorThreadRunner) - return; - QEventLoop loop; - vizCompositorThreadRunner->CleanupForShutdown(base::BindOnce(&QEventLoop::quit, base::Unretained(&loop))); - loop.exec(); - } - static void destroyGpuProcess() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); @@ -133,10 +119,8 @@ static std::unique_ptr<content::GpuThreadController> createGpuThreadController( } // static -void WebEngineContext::destroyGpuProcess(bool threaded) +void WebEngineContext::destroyGpuProcess() { - if (!threaded) - GpuThreadControllerQt::cleanupVizProcess(); GpuThreadControllerQt::destroyGpuProcess(); } |