Title: [263504] trunk/Source/WebCore
- Revision
- 263504
- Author
- ab...@igalia.com
- Date
- 2020-06-25 06:02:15 -0700 (Thu, 25 Jun 2020)
Log Message
[GStreamer] Don't invalidate MainThreadNotifier until the streaming threads are joined
https://bugs.webkit.org/show_bug.cgi?id=213197
Reviewed by Xabier Rodriguez-Calvar.
~MediaPlayerPrivateGStreamer() used to call m_notifier->invalidate()
before setting the pipeline to NULL state. This caused a race where
the streaming threads could post a task to the MainThreadNotifier
between these two events and cause a crash in
ASSERT(m_isValid.load()) -- that is, the notifier was used while
invalidated.
Fixing this is actually easy. ~MediaPlayerPrivateGStreamer() is always
run from the main thread, so no MainThreadNotifier tasks will be run
before the destructor is complete. By moving the
m_notifier->invalidate() call to after the pipeline has been set to
NULL (and therefore all streaming threads torn down) we are ensuring
no more taks will be posted, and since the MainThreadNotifier is
invalidated by that point, already posted tasks will not be run.
The race fixed by this patch is rare and wide-arching, so this patch
doesn't introduce TestExpectations changes, but it should avoid
further crashes like the ones reported in the Bugzilla issue attached.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (263503 => 263504)
--- trunk/Source/WebCore/ChangeLog 2020-06-25 10:49:42 UTC (rev 263503)
+++ trunk/Source/WebCore/ChangeLog 2020-06-25 13:02:15 UTC (rev 263504)
@@ -1,3 +1,32 @@
+2020-06-25 Alicia Boya García <ab...@igalia.com>
+
+ [GStreamer] Don't invalidate MainThreadNotifier until the streaming threads are joined
+ https://bugs.webkit.org/show_bug.cgi?id=213197
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ ~MediaPlayerPrivateGStreamer() used to call m_notifier->invalidate()
+ before setting the pipeline to NULL state. This caused a race where
+ the streaming threads could post a task to the MainThreadNotifier
+ between these two events and cause a crash in
+ ASSERT(m_isValid.load()) -- that is, the notifier was used while
+ invalidated.
+
+ Fixing this is actually easy. ~MediaPlayerPrivateGStreamer() is always
+ run from the main thread, so no MainThreadNotifier tasks will be run
+ before the destructor is complete. By moving the
+ m_notifier->invalidate() call to after the pipeline has been set to
+ NULL (and therefore all streaming threads torn down) we are ensuring
+ no more taks will be posted, and since the MainThreadNotifier is
+ invalidated by that point, already posted tasks will not be run.
+
+ The race fixed by this patch is rare and wide-arching, so this patch
+ doesn't introduce TestExpectations changes, but it should avoid
+ further crashes like the ones reported in the Bugzilla issue attached.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+ (WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer):
+
2020-06-23 Sergio Villar Senin <svil...@igalia.com>
[WebXR] Check device orientation support when requesting a reference space
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (263503 => 263504)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2020-06-25 10:49:42 UTC (rev 263503)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2020-06-25 13:02:15 UTC (rev 263504)
@@ -269,8 +269,6 @@
downcast<Nicosia::ContentLayerTextureMapperImpl>(m_nicosiaLayer->impl()).invalidateClient();
#endif
- m_notifier->invalidate();
-
if (m_videoSink)
g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
@@ -294,6 +292,7 @@
gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
m_player = nullptr;
+ m_notifier->invalidate();
}
bool MediaPlayerPrivateGStreamer::isAvailable()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
