[Webkit-unassigned] [Bug 203353] [WinCairo] GIFImageReader::m_data is ref/deref in different threads

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=203353

--- Comment #4 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 381798
  --> https://bugs.webkit.org/attachment.cgi?id=381798
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381798&action=review

> Source/WebCore/platform/image-decoders/gif/GIFImageReader.h:320
> -    RefPtr<WebCore::SharedBuffer> m_data;
> +    // GIFImageDecoder owns m_data. SharedBuffer is a thread-unsafe RefCounted object. Don't ref/deref in other threads.
> +    WebCore::SharedBuffer* m_data;

It is strange that we have the reader owns/has a reference to the SharedBuffer. The reader already has a pointer to the client which is GIFImageDecoder. And the GIFImageDecoder owns the "RefPtr<SharedBuffer> m_data" through the base class ScalableImageDecoder.

So I would suggest removing this member completely and access it through the client. Something like this:

    SharedBuffer* ScalableImageDecoder::data() { return m_data->get(); }

    SharedBuffer* GIFImageReader::data() { return m_client ? m_client->data() : nullptr; }

So all the instances of m_data in GIFImageReader.cpp can be replaced by data().

Also I think the same problem occurs with other decoders/readers but it is more obvious with GIF since we do many decoding. I would suggest reviewing the other readers and try to generalize the fix.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20191024/9eb29236/attachment-0001.htm>