Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock on macOS when running GStreamer in the main thread #86

Merged
merged 1 commit into from
Jul 6, 2024
Merged

Fix deadlock on macOS when running GStreamer in the main thread #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 41 additions & 15 deletions gstcefsrc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ run_cef (GstCefSrc *src)
static GstStateChangeReturn
gst_cef_src_change_state(GstElement *element, GstStateChange transition)
{
auto result = GST_CALL_PARENT_WITH_DEFAULT (GST_ELEMENT_CLASS , change_state, (element, transition), GST_STATE_CHANGE_FAILURE);
GstStateChangeReturn result = GST_STATE_CHANGE_SUCCESS;

switch(transition)
{
Expand Down Expand Up @@ -733,13 +733,20 @@ gst_cef_src_change_state(GstElement *element, GstStateChange transition)
cef_status = CEF_STATUS_SHUTTING_DOWN;
#ifdef __APPLE__
/* in the main thread as per Cocoa */
dispatch_async_f(dispatch_get_main_queue(), nullptr, (dispatch_function_t)&gst_cef_shutdown);
#endif
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*)element, (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);
#ifndef __APPLE__
g_thread_join(thread);
thread = nullptr;
#endif
Expand All @@ -750,6 +757,10 @@ gst_cef_src_change_state(GstElement *element, GstStateChange transition)
default:
break;
}

if (result == GST_STATE_CHANGE_FAILURE) return result;
result = GST_ELEMENT_CLASS(parent_class)->change_state(element, transition);

return result;
}

Expand Down Expand Up @@ -778,13 +789,22 @@ gst_cef_src_start(GstBaseSrc *base_src)
GST_OBJECT_UNLOCK (src);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The g_cond_wait() 10 lines above would also wait forever if we end up here on macOS on the main thread or not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not after this MR. If GStreamer is managed from the main thread, this function gets called by line 762 above, by which time either CEF has initialized (skipping the wait altogether), or it has failed (and this function will not be called at all). The wait is still necessary if the app handles CEF in a different event loop from the GStreamer pipeline's, like with gst-launch-1.0.


browserClient = new BrowserClient(renderHandler, audioHandler, requestHandler, displayHandler, src);
CefPostTask(TID_UI, base::BindOnce(&BrowserClient::MakeBrowser, browserClient.get(), 0));
#ifdef __APPLE__
if (pthread_main_np()) {
amyspark marked this conversation as resolved.
Show resolved Hide resolved
/* in the main thread as per Cocoa */
browserClient->MakeBrowser(0);
} else {
#endif
CefPostTask(TID_UI, base::BindOnce(&BrowserClient::MakeBrowser, browserClient.get(), 0));

/* And wait for this src's browser to have been created */
g_mutex_lock(&src->state_lock);
while (!src->started)
g_cond_wait (&src->state_cond, &src->state_lock);
g_mutex_unlock (&src->state_lock);
/* And wait for this src's browser to have been created */
g_mutex_lock(&src->state_lock);
while (!src->started)
g_cond_wait (&src->state_cond, &src->state_lock);
g_mutex_unlock (&src->state_lock);
#ifdef __APPLE__
}
#endif

ret = src->browser != NULL;

Expand All @@ -807,11 +827,17 @@ gst_cef_src_stop (GstBaseSrc *base_src)

if (src->browser) {
gst_cef_src_close_browser(src);
/* And wait for this src's browser to have been closed */
g_mutex_lock(&src->state_lock);
while (src->started)
g_cond_wait (&src->state_cond, &src->state_lock);
g_mutex_unlock (&src->state_lock);
#ifdef __APPLE__
if (!pthread_main_np()) {
#endif
/* And wait for this src's browser to have been closed */
g_mutex_lock(&src->state_lock);
while (src->started)
g_cond_wait (&src->state_cond, &src->state_lock);
g_mutex_unlock (&src->state_lock);
#ifdef __APPLE__
}
#endif
}

gst_buffer_replace (&src->current_buffer, NULL);
Expand Down
Loading