wpesrc: fix possible small deadlock on shutdown

Problem is that unreffing the EGLImage/SHM Buffer while holding the
images_mutex lock may deadlock when a new buffer is advertised and
an attempt is made to lock the images_mutex there.

The advertisement of the new image/buffer is performed in the
WPEContextThread and the blocking dispatch when unreffing wants to run
something on the WPEContextThread however images_mutex has already been
locked by the destructor.

Delay unreffing images/buffers outside of images_mutex and instead just
clear the relevant fields within the lock.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1843>
This commit is contained in:
Matthew Waters 2020-11-27 16:18:29 +11:00 committed by GStreamer Merge Bot
parent 1753d2931c
commit 3ed0ee95f2

View file

@ -292,6 +292,10 @@ WPEView::WPEView(WebKitWebContext* web_context, GstWpeSrc* src, GstGLContext* co
WPEView::~WPEView() WPEView::~WPEView()
{ {
GstEGLImage *egl_pending = NULL;
GstEGLImage *egl_committed = NULL;
GstBuffer *shm_pending = NULL;
GstBuffer *shm_committed = NULL;
GST_TRACE ("%p destroying", this); GST_TRACE ("%p destroying", this);
g_mutex_clear(&threading.ready_mutex); g_mutex_clear(&threading.ready_mutex);
@ -301,25 +305,34 @@ WPEView::~WPEView()
GMutexHolder lock(images_mutex); GMutexHolder lock(images_mutex);
if (egl.pending) { if (egl.pending) {
gst_egl_image_unref(egl.pending); egl_pending = egl.pending;
egl.pending = nullptr; egl.pending = nullptr;
} }
if (egl.committed) { if (egl.committed) {
gst_egl_image_unref(egl.committed); egl_committed = egl.committed;
egl.committed = nullptr; egl.committed = nullptr;
} }
if (shm.pending) { if (shm.pending) {
GST_TRACE ("%p freeing shm pending %" GST_PTR_FORMAT, this, shm.pending); GST_TRACE ("%p freeing shm pending %" GST_PTR_FORMAT, this, shm.pending);
gst_buffer_unref(shm.pending); shm_pending = shm.pending;
shm.pending = nullptr; shm.pending = nullptr;
} }
if (shm.committed) { if (shm.committed) {
GST_TRACE ("%p freeing shm commited %" GST_PTR_FORMAT, this, shm.committed); GST_TRACE ("%p freeing shm commited %" GST_PTR_FORMAT, this, shm.committed);
gst_buffer_unref(shm.committed); shm_committed = shm.committed;
shm.committed = nullptr; shm.committed = nullptr;
} }
} }
if (egl_pending)
gst_egl_image_unref (egl_pending);
if (egl_committed)
gst_egl_image_unref (egl_committed);
if (shm_pending)
gst_buffer_unref (shm_pending);
if (shm_committed)
gst_buffer_unref (shm_committed);
WPEContextThread::singleton().dispatch([&]() { WPEContextThread::singleton().dispatch([&]() {
if (webkit.view) { if (webkit.view) {
g_object_unref(webkit.view); g_object_unref(webkit.view);
@ -370,6 +383,7 @@ GstEGLImage* WPEView::image()
{ {
GstEGLImage* ret = nullptr; GstEGLImage* ret = nullptr;
bool dispatchFrameComplete = false; bool dispatchFrameComplete = false;
GstEGLImage *prev_image = NULL;
{ {
GMutexHolder lock(images_mutex); GMutexHolder lock(images_mutex);
@ -380,12 +394,10 @@ GstEGLImage* WPEView::image()
GST_IS_EGL_IMAGE(egl.committed) ? GST_MINI_OBJECT_REFCOUNT_VALUE(GST_MINI_OBJECT_CAST(egl.committed)) : 0); GST_IS_EGL_IMAGE(egl.committed) ? GST_MINI_OBJECT_REFCOUNT_VALUE(GST_MINI_OBJECT_CAST(egl.committed)) : 0);
if (egl.pending) { if (egl.pending) {
auto* previousImage = egl.committed; prev_image = egl.committed;
egl.committed = egl.pending; egl.committed = egl.pending;
egl.pending = nullptr; egl.pending = nullptr;
if (previousImage)
gst_egl_image_unref(previousImage);
dispatchFrameComplete = true; dispatchFrameComplete = true;
} }
@ -393,6 +405,9 @@ GstEGLImage* WPEView::image()
ret = egl.committed; ret = egl.committed;
} }
if (prev_image)
gst_egl_image_unref(prev_image);
if (dispatchFrameComplete) if (dispatchFrameComplete)
frameComplete(); frameComplete();
@ -403,6 +418,7 @@ GstBuffer* WPEView::buffer()
{ {
GstBuffer* ret = nullptr; GstBuffer* ret = nullptr;
bool dispatchFrameComplete = false; bool dispatchFrameComplete = false;
GstBuffer *prev_image = NULL;
{ {
GMutexHolder lock(images_mutex); GMutexHolder lock(images_mutex);
@ -413,12 +429,10 @@ GstBuffer* WPEView::buffer()
GST_IS_BUFFER(shm.committed) ? GST_MINI_OBJECT_REFCOUNT_VALUE(GST_MINI_OBJECT_CAST(shm.committed)) : 0); GST_IS_BUFFER(shm.committed) ? GST_MINI_OBJECT_REFCOUNT_VALUE(GST_MINI_OBJECT_CAST(shm.committed)) : 0);
if (shm.pending) { if (shm.pending) {
auto* previousImage = shm.committed; prev_image = shm.committed;
shm.committed = shm.pending; shm.committed = shm.pending;
shm.pending = nullptr; shm.pending = nullptr;
if (previousImage)
gst_buffer_unref(previousImage);
dispatchFrameComplete = true; dispatchFrameComplete = true;
} }
@ -426,6 +440,9 @@ GstBuffer* WPEView::buffer()
ret = shm.committed; ret = shm.committed;
} }
if (prev_image)
gst_buffer_unref(prev_image);
if (dispatchFrameComplete) if (dispatchFrameComplete)
frameComplete(); frameComplete();