[GStreamer] Some layout tests issue "g_mutex_clear() called on uninitialised or locke...
authormagomez@igalia.com <magomez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jul 2017 13:07:01 +0000 (13:07 +0000)
committermagomez@igalia.com <magomez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jul 2017 13:07:01 +0000 (13:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173952

Reviewed by Carlos Garcia Campos.

Adjust MediaPlayerPrivateGStreamerBase to avoid concurrence problems with the GStreamer thread when
destroying the object.

Covered by existent tests.

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
(WebCore::MediaPlayerPrivateGStreamerBase::repaint):
(WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219681 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp

index 18c93ab..853d8b2 100644 (file)
@@ -1,3 +1,20 @@
+2017-07-20  Miguel Gomez  <magomez@igalia.com>
+
+        [GStreamer] Some layout tests issue "g_mutex_clear() called on uninitialised or locked mutex" and flaky crash in ~MediaPlayerPrivateGStreamerBase
+        https://bugs.webkit.org/show_bug.cgi?id=173952
+
+        Reviewed by Carlos Garcia Campos.
+
+        Adjust MediaPlayerPrivateGStreamerBase to avoid concurrence problems with the GStreamer thread when
+        destroying the object.
+
+        Covered by existent tests.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+        (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
+        (WebCore::MediaPlayerPrivateGStreamerBase::repaint):
+        (WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint):
+
 2017-07-19  Zan Dobersek  <zdobersek@igalia.com>
 
         [EME] Push CDMInstance, CDMPrivate and associated types into the Platform layer
index 575d358..2932da5 100644 (file)
@@ -239,8 +239,6 @@ MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
 {
     m_notifier->invalidate();
 
-    cancelRepaint();
-
     if (m_videoSink) {
         g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
 #if USE(GSTREAMER_GL)
@@ -251,15 +249,20 @@ MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
 #endif
     }
 
-    g_mutex_clear(&m_sampleMutex);
-
-    m_player = nullptr;
-
     if (m_volumeElement)
         g_signal_handlers_disconnect_matched(m_volumeElement.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
 
+    // This will release the GStreamer thread from m_drawCondition if AC is disabled.
+    cancelRepaint();
+
+    // The change to GST_STATE_NULL state is always synchronous. So after this gets executed we don't need to worry
+    // about handlers running in the GStreamer thread.
     if (m_pipeline)
         gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
+
+    g_mutex_clear(&m_sampleMutex);
+
+    m_player = nullptr;
 }
 
 void MediaPlayerPrivateGStreamerBase::setPipeline(GstElement* pipeline)
@@ -626,15 +629,8 @@ void MediaPlayerPrivateGStreamerBase::repaint()
 
     m_player->repaint();
 
-#if USE(GSTREAMER_GL)
-    bool shouldNotifyDraw = !m_renderingCanBeAccelerated;
-#else
-    bool shouldNotifyDraw = true;
-#endif
-    if (shouldNotifyDraw) {
-        LockHolder lock(m_drawMutex);
-        m_drawCondition.notifyOne();
-    }
+    LockHolder lock(m_drawMutex);
+    m_drawCondition.notifyOne();
 }
 
 void MediaPlayerPrivateGStreamerBase::triggerRepaint(GstSample* sample)
@@ -679,12 +675,7 @@ void MediaPlayerPrivateGStreamerBase::repaintCallback(MediaPlayerPrivateGStreame
 
 void MediaPlayerPrivateGStreamerBase::cancelRepaint()
 {
-#if USE(GSTREAMER_GL)
-    bool shouldCancelRepaint = !m_renderingCanBeAccelerated;
-#else
-    bool shouldCancelRepaint = true;
-#endif
-    if (shouldCancelRepaint) {
+    if (!m_renderingCanBeAccelerated) {
         m_drawTimer.stop();
         LockHolder locker(m_drawMutex);
         m_drawCondition.notifyOne();