Changeset 210501 in webkit

Timestamp:

Jan 9, 2017 4:12:14 AM (7 years ago)

Author:

Carlos Garcia Campos

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/image-decoders/ImageDecoder.cpp (modified) (3 diffs)
• platform/image-decoders/ImageDecoder.h (modified) (2 diffs)
• platform/image-decoders/gif/GIFImageDecoder.cpp (modified) (1 diff)
• platform/image-decoders/gif/GIFImageDecoder.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-01-09  Carlos Garcia Campos  <cgarcia@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  <alex@igalia.com>
 

trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp

@@ -171 +171 @@
 bool ImageDecoder::frameIsCompleteAtIndex(size_t index)
 {
-    LockHolder locker(m_lock);
     ImageFrame* buffer = frameBufferAtIndex(index);
     return buffer && buffer->isComplete();
@@ -195 +194 @@
 float ImageDecoder::frameDurationAtIndex(size_t index)
 {
-    LockHolder locker(m_lock);
     ImageFrame* buffer = frameBufferAtIndex(index);
     if (!buffer || buffer->isEmpty())
@@ -216 +214 @@
         return nullptr;
 
-    LockHolder locker(m_lock);
     ImageFrame* buffer = frameBufferAtIndex(index);
     if (!buffer || buffer->isEmpty() || !buffer->hasBackingStore())

trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h

@@ -35 +35 @@
 #include "SharedBuffer.h"
 #include <wtf/Assertions.h>
-#include <wtf/Lock.h>
 #include <wtf/Optional.h>
 #include <wtf/RefPtr.h>
@@ -216 +215 @@
         bool m_isAllDataReceived { false };
         bool m_failed { false };
-        Lock m_lock;
     };
 

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

@@ -307 +307 @@
         return;
 
+    LockHolder locker(m_decodeLock);
     if (!m_reader) {
         m_reader = std::make_unique<GIFImageReader>(this);

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h

@@ -27 +27 @@
 
 #include "ImageDecoder.h"
+#include <wtf/Lock.h>
 
 class GIFImageReader;
@@ -74 +75 @@
         mutable RepetitionCount m_repetitionCount { RepetitionCountOnce };
         std::unique_ptr<GIFImageReader> m_reader;
+        Lock m_decodeLock;
     };