[Webkit-unassigned] [Bug 167304] [GTK] WebProcess from WebKitGtk+ 2.15.x SIGSEVs in GIFLZWContext::doLZW(unsigned char const*, unsigned long) at Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:303

[bugzilla-daemon at webkit.org]

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

--- Comment #11 from Miguel Gomez <magomez at igalia.com> ---
More details.

There are actually 2 different crashes, both of them happening because something unexpected happens while there's a decoding ongoing on the decoding thread.

* The fist one affects only WebKitGtk+ code. It happens because GIFImageDecoder::clearFrameBufferCache() gets called, and that function deletes the GIFImageReader that is being used for decoding in the decoding thread. I'll upload a patch that avoid this.

* The second crash affects multiplatform code, and it happens because the decoder being used in the decoding thread can be deleted while there's still decoding happening.
The decoder is a unique_ptr stored in ImageSource, and that class is the one that sets it to ImageFrameCache, which is the one using it (it's stored as a simple pointer in ImageFrameCache).
When ImageSource::clear() gets called, it sets the ImageFrameCache decoder to null, without ensuring somehow that it's not being used at that point by the decoding thread, and this causes the crash.

I see 2 ways to fix this:

* Using a lock to ensure that ImageFrameCache::stopAsyncDecodingQueue() doesn't return until the decoding thread has stopped processing the requested frames. That function gets called before ImageSource::clear() and its mission is to stop the decoding in the secondary thread, but currently it's possible that there's a frame being decoded when that function returns. If we add a lock there and in the function that performs the decoding in the secondary thread, we can be sure that there's no decoding happening and we can delete the decoder safely. I've tested this fix and it seems to work properly.

* Another option is to turn the decoder into a RefPtr inside ImageSource, pass that RefPtr to ImageFrameCache and use it inside the function that performs the decoding in the secondary thread. This way ImageSource can unref the decoder when it wants, but it won't be destroyed until the decoding thread finishes using it.

Which do you think is the best option?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170306/52b25775/attachment.html>