Title: [264816] trunk
- Revision
- 264816
- Author
- [email protected]
- Date
- 2020-07-24 03:43:43 -0700 (Fri, 24 Jul 2020)
Log Message
[GStreamer] gst_buffer_unmap: assertion 'GST_IS_BUFFER (buffer)' failed
https://bugs.webkit.org/show_bug.cgi?id=213364
Reviewed by Philippe Normand.
Source/WebCore:
The initial patch for this bug had good intentions but it failed
in something basic and tests did not tackle the original
issue. The problem was in that the destructor of the subclass was
being called before the destructor of the superclass, hence the
buffer was unreffed before unmapped.
Now we finish what we started by adding an unmapEarly method that
can be used as regular API if wanted and that it is used by the
GstMappedOwnedBuffer destructor to instruct GstMappedBuffer
superclass to actually unmap without having to wait for the
destructor of the superclass to kick in.
Tests: GStreamerTest::mappedOwnedBufferDestruction and
GStreamerTest::mappedBufferValidityUnmapEarly API tests.
* platform/graphics/gstreamer/GStreamerCommon.h:
(WebCore::GstMappedBuffer::~GstMappedBuffer):
(WebCore::GstMappedBuffer::unmapEarly):
(WebCore::GstMappedOwnedBuffer::~GstMappedOwnedBuffer):
Tools:
* TestWebKitAPI/Tests/WebCore/gstreamer/GstMappedBuffer.cpp:
(TestWebKitAPI::TEST_F): Added
GStreamerTest::mappedOwnedBufferDestruction and
GStreamerTest::mappedBufferValidityUnmapEarly tests.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (264815 => 264816)
--- trunk/Source/WebCore/ChangeLog 2020-07-24 10:06:36 UTC (rev 264815)
+++ trunk/Source/WebCore/ChangeLog 2020-07-24 10:43:43 UTC (rev 264816)
@@ -1,3 +1,30 @@
+2020-07-24 Xabier Rodriguez Calvar <[email protected]>
+
+ [GStreamer] gst_buffer_unmap: assertion 'GST_IS_BUFFER (buffer)' failed
+ https://bugs.webkit.org/show_bug.cgi?id=213364
+
+ Reviewed by Philippe Normand.
+
+ The initial patch for this bug had good intentions but it failed
+ in something basic and tests did not tackle the original
+ issue. The problem was in that the destructor of the subclass was
+ being called before the destructor of the superclass, hence the
+ buffer was unreffed before unmapped.
+
+ Now we finish what we started by adding an unmapEarly method that
+ can be used as regular API if wanted and that it is used by the
+ GstMappedOwnedBuffer destructor to instruct GstMappedBuffer
+ superclass to actually unmap without having to wait for the
+ destructor of the superclass to kick in.
+
+ Tests: GStreamerTest::mappedOwnedBufferDestruction and
+ GStreamerTest::mappedBufferValidityUnmapEarly API tests.
+
+ * platform/graphics/gstreamer/GStreamerCommon.h:
+ (WebCore::GstMappedBuffer::~GstMappedBuffer):
+ (WebCore::GstMappedBuffer::unmapEarly):
+ (WebCore::GstMappedOwnedBuffer::~GstMappedOwnedBuffer):
+
2020-07-23 Alex Christensen <[email protected]>
Split Windows parts from CFNetworkSPI.h to CFNetworkSPIWin.h
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h (264815 => 264816)
--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h 2020-07-24 10:06:36 UTC (rev 264815)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h 2020-07-24 10:43:43 UTC (rev 264816)
@@ -113,6 +113,11 @@
virtual ~GstMappedBuffer()
{
+ unmapEarly();
+ }
+
+ void unmapEarly()
+ {
if (m_isValid) {
m_isValid = false;
gst_buffer_unmap(m_buffer, &m_info);
@@ -167,6 +172,11 @@
return create(GRefPtr(buffer));
}
+ virtual ~GstMappedOwnedBuffer()
+ {
+ unmapEarly();
+ }
+
Ref<SharedBuffer> createSharedBuffer();
private:
Modified: trunk/Tools/ChangeLog (264815 => 264816)
--- trunk/Tools/ChangeLog 2020-07-24 10:06:36 UTC (rev 264815)
+++ trunk/Tools/ChangeLog 2020-07-24 10:43:43 UTC (rev 264816)
@@ -1,3 +1,15 @@
+2020-07-24 Xabier Rodriguez Calvar <[email protected]>
+
+ [GStreamer] gst_buffer_unmap: assertion 'GST_IS_BUFFER (buffer)' failed
+ https://bugs.webkit.org/show_bug.cgi?id=213364
+
+ Reviewed by Philippe Normand.
+
+ * TestWebKitAPI/Tests/WebCore/gstreamer/GstMappedBuffer.cpp:
+ (TestWebKitAPI::TEST_F): Added
+ GStreamerTest::mappedOwnedBufferDestruction and
+ GStreamerTest::mappedBufferValidityUnmapEarly tests.
+
2020-07-24 Angelos Oikonomopoulos <[email protected]>
Add a GNU parallel runner
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/gstreamer/GstMappedBuffer.cpp (264815 => 264816)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/gstreamer/GstMappedBuffer.cpp 2020-07-24 10:06:36 UTC (rev 264815)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/gstreamer/GstMappedBuffer.cpp 2020-07-24 10:43:43 UTC (rev 264816)
@@ -93,6 +93,15 @@
ASSERT_EQ(GST_OBJECT_REFCOUNT(buf.get()), 1);
}
+TEST_F(GStreamerTest, mappedBufferValidityUnmapEarly)
+{
+ GRefPtr<GstBuffer> buf = adoptGRef(gst_buffer_new());
+ GstMappedBuffer mappedBuf(buf, GST_MAP_READ);
+ ASSERT_TRUE(mappedBuf);
+ mappedBuf.unmapEarly();
+ ASSERT_FALSE(mappedBuf);
+}
+
TEST_F(GStreamerTest, mappedOwnedBufferReadSanity)
{
gpointer memory = g_malloc(16);
@@ -134,6 +143,33 @@
ASSERT_EQ(GST_OBJECT_REFCOUNT(buf.get()), 1);
}
+TEST_F(GStreamerTest, mappedOwnedBufferDestruction)
+{
+ GstBuffer* buffer = gst_buffer_new();
+ ASSERT_EQ(GST_OBJECT_REFCOUNT(buffer), 1);
+ auto mappedBuffer = GstMappedOwnedBuffer::create(buffer);
+ ASSERT_TRUE(mappedBuffer);
+ ASSERT_EQ(GST_OBJECT_REFCOUNT(buffer), 2);
+
+ // After this we should not use the buffer anymore but it should
+ // be still alive inside the mapped buffer, so we still can check
+ // the refcount.
+ gst_buffer_unref(buffer);
+ ASSERT_EQ(GST_OBJECT_REFCOUNT(buffer), 1);
+
+ // Now we are going to check if the buffer really outlives the
+ // mapped buffer. For that we check if there is no stderr output
+ // when we unref the mapped buffer. We can't check the ref count
+ // because it should be 0 and the object should be invalid, what
+ // could lead to a crash or other bad things.
+ char capturedStderrBuffer[BUFSIZ];
+ capturedStderrBuffer[0] = 0;
+ setbuf(stderr, capturedStderrBuffer);
+ mappedBuffer = nullptr;
+ setbuf(stderr, nullptr);
+ ASSERT_EQ(capturedStderrBuffer[0], 0);
+}
+
} // namespace TestWebKitAPI
#endif // USE(GSTREAMER)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
