Title: [231415] releases/WebKitGTK/webkit-2.20/Source/WebCore
- Revision
- 231415
- Author
- [email protected]
- Date
- 2018-05-07 01:18:48 -0700 (Mon, 07 May 2018)
Log Message
Merge r230629 - REGRESSION(r230627): [GTK][WPE] Possible deadlock when destroying the player in non AC mode
https://bugs.webkit.org/show_bug.cgi?id=184583
Reviewed by Carlos Garcia Campos.
In non AC mode, ensure that a deadlock can't happen when destroying MediaPlayerPrivateGStreamerBase.
Covered by existent tests.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
(WebCore::MediaPlayerPrivateGStreamerBase::triggerRepaint):
(WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint):
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (231414 => 231415)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-05-07 08:18:44 UTC (rev 231414)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-05-07 08:18:48 UTC (rev 231415)
@@ -1,5 +1,22 @@
2018-04-13 Miguel Gomez <[email protected]>
+ REGRESSION(r230627): [GTK][WPE] Possible deadlock when destroying the player in non AC mode
+ https://bugs.webkit.org/show_bug.cgi?id=184583
+
+ Reviewed by Carlos Garcia Campos.
+
+ In non AC mode, ensure that a deadlock can't happen when destroying MediaPlayerPrivateGStreamerBase.
+
+ Covered by existent tests.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+ (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
+ (WebCore::MediaPlayerPrivateGStreamerBase::triggerRepaint):
+ (WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint):
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
+
+2018-04-13 Miguel Gomez <[email protected]>
+
[GTK] [gstreamer] video won't unpause when built with -DUSE_GSTREAMER_GL=OFF
https://bugs.webkit.org/show_bug.cgi?id=183362
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (231414 => 231415)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2018-05-07 08:18:44 UTC (rev 231414)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2018-05-07 08:18:48 UTC (rev 231415)
@@ -241,9 +241,6 @@
MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
{
- // Flag the player as being destroyed, so triggerRepaint will ignore incoming samples.
- m_destroying = true;
-
#if ENABLE(ENCRYPTED_MEDIA)
m_protectionCondition.notifyAll();
#endif
@@ -262,8 +259,9 @@
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 in non AC mode in case there's an ongoing triggerRepaint call.
- cancelRepaint();
+ // This will release the GStreamer thread from m_drawCondition in non AC mode in case there's an ongoing triggerRepaint call
+ // waiting there, and ensure that any triggerRepaint call reaching the lock won't wait on m_drawCondition.
+ cancelRepaint(true);
// 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.
@@ -792,10 +790,6 @@
void MediaPlayerPrivateGStreamerBase::triggerRepaint(GstSample* sample)
{
- // Do not try to process new frames if the player is being destroyed by the main thread.
- if (m_destroying)
- return;
-
bool triggerResize;
{
WTF::GMutexLocker<GMutex> lock(m_sampleMutex);
@@ -810,6 +804,8 @@
if (!m_renderingCanBeAccelerated) {
LockHolder locker(m_drawMutex);
+ if (m_destroying)
+ return;
m_drawTimer.startOneShot(0_s);
m_drawCondition.wait(m_drawMutex);
return;
@@ -834,7 +830,7 @@
player->triggerRepaint(sample);
}
-void MediaPlayerPrivateGStreamerBase::cancelRepaint()
+void MediaPlayerPrivateGStreamerBase::cancelRepaint(bool destroying)
{
// The goal of this function is to release the GStreamer thread from m_drawCondition in triggerRepaint() in non-AC case,
// to avoid a deadlock if the player gets paused while waiting for drawing (see https://bugs.webkit.org/show_bug.cgi?id=170003):
@@ -841,9 +837,13 @@
// the main thread is waiting for the GStreamer thread to pause, but the GStreamer thread is locked waiting for the
// main thread to draw. This deadlock doesn't happen when using AC because the sample is processed (not painted) in the compositor
// thread, so the main thread can request the pause and wait if the GStreamer thread is waiting for the compositor thread.
+ //
+ // This function is also used when destroying the player (destroying parameter is true), to release the gstreamer thread from
+ // m_drawCondition and to ensure that new triggerRepaint calls won't wait on m_drawCondition.
if (!m_renderingCanBeAccelerated) {
+ LockHolder locker(m_drawMutex);
m_drawTimer.stop();
- LockHolder locker(m_drawMutex);
+ m_destroying = destroying;
m_drawCondition.notifyOne();
}
}
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h (231414 => 231415)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2018-05-07 08:18:44 UTC (rev 231414)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2018-05-07 08:18:48 UTC (rev 231415)
@@ -190,7 +190,7 @@
void triggerRepaint(GstSample*);
void repaint();
- void cancelRepaint();
+ void cancelRepaint(bool destroying = false);
static void repaintCallback(MediaPlayerPrivateGStreamerBase*, GstSample*);
static void repaintCancelledCallback(MediaPlayerPrivateGStreamerBase*);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
