[Webkit-unassigned] [Bug 169199] New: ImageDecoder can be deleted while the async decoder thread is still using it

[bugzilla-daemon at webkit.org]

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

            Bug ID: 169199
           Summary: ImageDecoder can be deleted while the async decoder
                    thread is still using it
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Local Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Keywords: Gtk
          Severity: Normal
          Priority: P2
         Component: Platform
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: cgarcia at igalia.com
                CC: bugs-noreply at webkitgtk.org, magomez at igalia.com,
                    sabouhallawa at apple.com

As explained by Miguel in bug #167304

"* 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/2139f1f6/attachment-0001.html>