Title: [210501] trunk/Source/WebCore
Revision
210501
Author
carlo...@webkit.org
Date
2017-01-09 04:12:14 -0800 (Mon, 09 Jan 2017)

Log Message

[GTK] WebProcess from WebKitGtk+ 2.15.2 SIGSEGVs in std::unique_ptr<SoupBuffer, WTF::GPtrDeleter<SoupBuffer> >::get() const () at /usr/include/c++/6/bits/unique_ptr.h:305
https://bugs.webkit.org/show_bug.cgi?id=165848

Reviewed by Michael Catanzaro.

In r208881 several locks were added to ImageDecoder to prevent frameBufferAtIndex() from being called by multiple
threads at the same time, but I forgot isSizeAvailable() also calls frameBufferAtIndex(). However, what we
really need to protect is the GIFImageDecoder, to never allow decoding from more than one thread at the same
time. This patch reverts r208881 and adds a lock to GIFImageDecoder::decode() instead.

* platform/image-decoders/ImageDecoder.cpp:
(WebCore::ImageDecoder::frameIsCompleteAtIndex):
(WebCore::ImageDecoder::frameDurationAtIndex):
(WebCore::ImageDecoder::createFrameImageAtIndex):
* platform/image-decoders/ImageDecoder.h:
* platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::decode):
* platform/image-decoders/gif/GIFImageDecoder.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (210500 => 210501)


--- trunk/Source/WebCore/ChangeLog	2017-01-09 11:25:49 UTC (rev 210500)
+++ trunk/Source/WebCore/ChangeLog	2017-01-09 12:12:14 UTC (rev 210501)
@@ -1,3 +1,24 @@
+2017-01-09  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK] WebProcess from WebKitGtk+ 2.15.2 SIGSEGVs in std::unique_ptr<SoupBuffer, WTF::GPtrDeleter<SoupBuffer> >::get() const () at /usr/include/c++/6/bits/unique_ptr.h:305
+        https://bugs.webkit.org/show_bug.cgi?id=165848
+
+        Reviewed by Michael Catanzaro.
+
+        In r208881 several locks were added to ImageDecoder to prevent frameBufferAtIndex() from being called by multiple
+        threads at the same time, but I forgot isSizeAvailable() also calls frameBufferAtIndex(). However, what we
+        really need to protect is the GIFImageDecoder, to never allow decoding from more than one thread at the same
+        time. This patch reverts r208881 and adds a lock to GIFImageDecoder::decode() instead.
+
+        * platform/image-decoders/ImageDecoder.cpp:
+        (WebCore::ImageDecoder::frameIsCompleteAtIndex):
+        (WebCore::ImageDecoder::frameDurationAtIndex):
+        (WebCore::ImageDecoder::createFrameImageAtIndex):
+        * platform/image-decoders/ImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::decode):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+
 2017-01-09  Alejandro G. Castro  <a...@igalia.com>
 
         [OWR] Unskip fast/mediastream/MediaStream-video-element-track-stop.html

Modified: trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp (210500 => 210501)


--- trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp	2017-01-09 11:25:49 UTC (rev 210500)
+++ trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp	2017-01-09 12:12:14 UTC (rev 210501)
@@ -170,7 +170,6 @@
 
 bool ImageDecoder::frameIsCompleteAtIndex(size_t index)
 {
-    LockHolder locker(m_lock);
     ImageFrame* buffer = frameBufferAtIndex(index);
     return buffer && buffer->isComplete();
 }
@@ -194,7 +193,6 @@
 
 float ImageDecoder::frameDurationAtIndex(size_t index)
 {
-    LockHolder locker(m_lock);
     ImageFrame* buffer = frameBufferAtIndex(index);
     if (!buffer || buffer->isEmpty())
         return 0;
@@ -215,7 +213,6 @@
     if (size().isEmpty())
         return nullptr;
 
-    LockHolder locker(m_lock);
     ImageFrame* buffer = frameBufferAtIndex(index);
     if (!buffer || buffer->isEmpty() || !buffer->hasBackingStore())
         return nullptr;

Modified: trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h (210500 => 210501)


--- trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h	2017-01-09 11:25:49 UTC (rev 210500)
+++ trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h	2017-01-09 12:12:14 UTC (rev 210501)
@@ -34,7 +34,6 @@
 #include "PlatformScreen.h"
 #include "SharedBuffer.h"
 #include <wtf/Assertions.h>
-#include <wtf/Lock.h>
 #include <wtf/Optional.h>
 #include <wtf/RefPtr.h>
 #include <wtf/Vector.h>
@@ -215,7 +214,6 @@
 #endif
         bool m_isAllDataReceived { false };
         bool m_failed { false };
-        Lock m_lock;
     };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp (210500 => 210501)


--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp	2017-01-09 11:25:49 UTC (rev 210500)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp	2017-01-09 12:12:14 UTC (rev 210501)
@@ -306,6 +306,7 @@
     if (failed())
         return;
 
+    LockHolder locker(m_decodeLock);
     if (!m_reader) {
         m_reader = std::make_unique<GIFImageReader>(this);
         m_reader->setData(m_data.get());

Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h (210500 => 210501)


--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h	2017-01-09 11:25:49 UTC (rev 210500)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h	2017-01-09 12:12:14 UTC (rev 210501)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "ImageDecoder.h"
+#include <wtf/Lock.h>
 
 class GIFImageReader;
 
@@ -73,6 +74,7 @@
         bool m_currentBufferSawAlpha;
         mutable RepetitionCount m_repetitionCount { RepetitionCountOnce };
         std::unique_ptr<GIFImageReader> m_reader;
+        Lock m_decodeLock;
     };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to