From 6f6b49d1c575a48941124d7034c5561c98a4fc29 Mon Sep 17 00:00:00 2001 From: Mathieu Duponchelle Date: Fri, 4 Oct 2024 16:17:03 +0200 Subject: [PATCH] cefsrc: fix CefInitialize crashing the second time Calling CefInitialize multiple times during a process lifetime is not supported: * https://magpcss.org/ceforum/viewtopic.php?f=6&t=16430 * https://www.magpcss.org/ceforum/viewtopic.php?f=6&t=466 * https://stackoverflow.com/questions/73172139/cef-can-only-be-initialized-once-per-process-this-is-a-limitation-of-the-underl Prior to #83 we used to have a ShutdownEnforcer class, but as it was causing issues on OSX we will simply no longer shut down CEF at all. We can revisit if needed, but this is preferable to crashing. See https://github.com/centricular/gstcefsrc/pull/88#issuecomment-2388711778 and below --- gstcefsrc.cc | 79 ++++------------------------------------------------ 1 file changed, 5 insertions(+), 74 deletions(-) diff --git a/gstcefsrc.cc b/gstcefsrc.cc index 57d9452..8ba9c39 100644 --- a/gstcefsrc.cc +++ b/gstcefsrc.cc @@ -62,19 +62,12 @@ using CefStatus = enum : guint8 { CEF_STATUS_INITIALIZED = 1U << 2U, // No CEF elements will be allowed to complete initialization. CEF_STATUS_FAILURE = 1U << 3U, - // CEF has been marked for shutdown, stopping any leftover events from being - // processed. Any elements that need it must wait till this flag and - // cef_initializing are cleared. - CEF_STATUS_SHUTTING_DOWN = 1U << 4U, }; + static CefStatus cef_status = CEF_STATUS_NOT_LOADED; static const guint8 CEF_STATUS_MASK_INITIALIZED = CEF_STATUS_FAILURE | CEF_STATUS_INITIALIZED; -static const guint8 CEF_STATUS_MASK_TRANSITIONING = - CEF_STATUS_SHUTTING_DOWN | CEF_STATUS_INITIALIZING; +static const guint8 CEF_STATUS_MASK_TRANSITIONING = CEF_STATUS_INITIALIZING; -// Number of running CEF instances. Setting this to 0 must be accompanied -// with cef_shutdown to prevent leaks on application exit. -static guint64 browsers = 0U; static GMutex init_lock; static GCond init_cond; @@ -472,13 +465,6 @@ class BrowserClient : src->state = CEF_SRC_CLOSED; g_cond_signal (&src->state_cond); g_mutex_unlock(&src->state_lock); - g_mutex_lock(&init_lock); - g_assert (browsers > 0); - browsers -= 1; - if (browsers == 0) { - CefQuitMessageLoop(); - } - g_mutex_unlock (&init_lock); } // CefRequestHandler methods: @@ -520,10 +506,6 @@ class BrowserClient : nullptr, nullptr ); - g_mutex_lock (&init_lock); - g_assert (browsers < G_MAXUINT64); - browsers += 1; - g_mutex_unlock(&init_lock); browser->GetHost()->SetAudioMuted(true); @@ -572,8 +554,6 @@ void BrowserApp::OnScheduleMessagePumpWork(int64_t delay_ms) workTimer_ = nullptr; } - if (cef_status == CEF_STATUS_SHUTTING_DOWN) return; - if (delay_ms <= 0) { // Execute the work immediately. gst_cef_loop(); @@ -675,21 +655,6 @@ static GstFlowReturn gst_cef_src_create(GstPushSrc *push_src, GstBuffer **buf) return GST_FLOW_OK; } -static void* -gst_cef_shutdown(void *) -{ -#ifdef __APPLE__ - CefQuitMessageLoop(); -#endif - - g_mutex_lock (&init_lock); - CefShutdown(); - cef_status = CEF_STATUS_NOT_LOADED; - g_cond_broadcast (&init_cond); - g_mutex_unlock (&init_lock); - return nullptr; -} - /* Once we have started a first cefsrc for this process, we start * a UI thread and never shut it down. We could probably refine this * to stop and restart the thread as needed, but this updated approach @@ -808,7 +773,6 @@ run_cef (GstCefSrc *src) g_mutex_unlock (&init_lock); #ifndef __APPLE__ CefRunMessageLoop(); - gst_cef_shutdown(nullptr); #endif done: @@ -863,38 +827,6 @@ gst_cef_src_change_state(GstElement *src, GstStateChange transition) g_mutex_unlock(&init_lock); break; } - case GST_STATE_CHANGE_READY_TO_NULL: - { - g_mutex_lock (&init_lock); - while (cef_status & CEF_STATUS_MASK_TRANSITIONING) - g_cond_wait (&init_cond, &init_lock); - /* At that point the element holds the lock and knows no status change is happening */ - if (browsers == 0 && cef_status == CEF_STATUS_INITIALIZED) { - /* This element is the one in charge of shutting down */ - cef_status = CEF_STATUS_SHUTTING_DOWN; -#ifdef __APPLE__ - /* in the main thread as per Cocoa */ - if (pthread_main_np()) { - g_mutex_unlock (&init_lock); - gst_cef_shutdown (nullptr); - g_mutex_lock (&init_lock); - } else { - dispatch_async_f(dispatch_get_main_queue(), (GstCefSrc*)src, (dispatch_function_t)&gst_cef_shutdown); - while (cef_status == CEF_STATUS_SHUTTING_DOWN) - g_cond_wait (&init_cond, &init_lock); - } -#else - // the UI thread handles it through the message loop return, - // this MUST NOT let GStreamer conduct unwind ops until CEF is truly dead - while (cef_status == CEF_STATUS_SHUTTING_DOWN) - g_cond_wait (&init_cond, &init_lock); - g_thread_join(thread); - thread = nullptr; -#endif - } - g_mutex_unlock (&init_lock); - break; - } default: break; } @@ -914,7 +846,6 @@ gst_cef_src_start(GstBaseSrc *base_src) GST_ELEMENT_PROGRESS(src, START, "open", ("Creating CEF browser client")); CefRefPtr browserClient = new BrowserClient(src); - gulong browser_id = browsers; /* Make sure CEF is initialized before posting a task */ g_mutex_lock (&init_lock); @@ -931,7 +862,7 @@ gst_cef_src_start(GstBaseSrc *base_src) src->n_frames = 0; GST_OBJECT_UNLOCK (src); - GST_ELEMENT_PROGRESS(src, CONTINUE, "open", ("Creating CEF browser (#%lu)...", browser_id)); + GST_ELEMENT_PROGRESS(src, CONTINUE, "open", ("Creating CEF browser ...")); #ifdef __APPLE__ if (pthread_main_np()) { @@ -965,12 +896,12 @@ gst_cef_src_start(GstBaseSrc *base_src) if (ret) { GST_ELEMENT_PROGRESS( src, COMPLETE, "open", - ("CEF browser created (#%lu)", browser_id) + ("CEF browser created") ); } else { GST_ELEMENT_PROGRESS( src, ERROR, "open", - ("CEF browser failed to create (#%lu)", browser_id) + ("CEF browser failed to create") ); }