Changeset 219681 in webkit

Timestamp:

Jul 20, 2017 6:07:01 AM (7 years ago)

Author:

magomez@igalia.com

Message:

[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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

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

@@ -240 +240 @@
     m_notifier->invalidate();
 
-    cancelRepaint();
-
     if (m_videoSink) {
         g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
@@ -252 +250 @@
     }
 
-    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;
 }
 
@@ -627 +630 @@
     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();
 }
 
@@ -680 +676 @@
 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);