Potentially non thread-safe usage of WebCore::MediaSample
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Aug 2019 02:20:54 +0000 (02:20 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Aug 2019 02:20:54 +0000 (02:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200734

Reviewed by Eric Carlson.

ImageSource is a main thread object but ends up getting passed to a background queue for image
decoding. In some cases, the background queue ends up being the last one holding a ref to the
ImageSource which ends up destroying the ImageSource on a background thread. Doing so is not
safe as shown by the crash.

To address the issue, have ImageSource subclass ThreadSafeRefCounted<ImageSource, WTF::DestructionThread::Main>
so that it is always destroyed on the main thread.

No new tests, currently crashing on the debug bots.

* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::ImageSource):
(WebCore::ImageSource::~ImageSource):
* platform/graphics/ImageSource.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248703 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/graphics/ImageSource.h

index b05e320..a3fb68b 100644 (file)
@@ -1,3 +1,25 @@
+2019-08-14  Chris Dumez  <cdumez@apple.com>
+
+        Potentially non thread-safe usage of WebCore::MediaSample
+        https://bugs.webkit.org/show_bug.cgi?id=200734
+
+        Reviewed by Eric Carlson.
+
+        ImageSource is a main thread object but ends up getting passed to a background queue for image
+        decoding. In some cases, the background queue ends up being the last one holding a ref to the
+        ImageSource which ends up destroying the ImageSource on a background thread. Doing so is not
+        safe as shown by the crash.
+
+        To address the issue, have ImageSource subclass ThreadSafeRefCounted<ImageSource, WTF::DestructionThread::Main>
+        so that it is always destroyed on the main thread.
+
+        No new tests, currently crashing on the debug bots.
+
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::ImageSource):
+        (WebCore::ImageSource::~ImageSource):
+        * platform/graphics/ImageSource.h:
+
 2019-08-14  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r248526.
index 752c474..18976f9 100644 (file)
@@ -48,10 +48,13 @@ ImageSource::ImageSource(BitmapImage* image, AlphaOption alphaOption, GammaAndCo
     , m_alphaOption(alphaOption)
     , m_gammaAndColorProfileOption(gammaAndColorProfileOption)
 {
+    ASSERT(isMainThread());
 }
 
 ImageSource::ImageSource(NativeImagePtr&& nativeImage)
 {
+    ASSERT(isMainThread());
+
     m_frameCount = 1;
     m_encodedDataStatus = EncodedDataStatus::Complete;
     growFrames();
@@ -69,6 +72,7 @@ ImageSource::ImageSource(NativeImagePtr&& nativeImage)
 ImageSource::~ImageSource()
 {
     ASSERT(!hasAsyncDecodingQueue());
+    ASSERT(isMainThread());
 }
 
 bool ImageSource::ensureDecoderAvailable(SharedBuffer* data)
index c05c11d..bc79099 100644 (file)
@@ -40,7 +40,7 @@ class BitmapImage;
 class GraphicsContext;
 class ImageDecoder;
 
-class ImageSource : public ThreadSafeRefCounted<ImageSource>, public CanMakeWeakPtr<ImageSource> {
+class ImageSource : public ThreadSafeRefCounted<ImageSource, WTF::DestructionThread::Main>, public CanMakeWeakPtr<ImageSource> {
     friend class BitmapImage;
 public:
     ~ImageSource();