Bug 172563 - Ensure ImageFrameCache does not access its BitmapImage after it is deleted
Summary: Ensure ImageFrameCache does not access its BitmapImage after it is deleted
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-24 17:30 PDT by Said Abou-Hallawa
Modified: 2017-05-25 11:22 PDT (History)
4 users (show)

See Also:

Attachments
Patch (5.84 KB, patch)
2017-05-24 17:47 PDT, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.15 KB, patch)
2017-05-24 18:34 PDT, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.31 KB, patch)
2017-05-25 10:59 PDT, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2017-05-24 17:30:20 PDT 
ImageFrameCache keeps a raw pointer to its container BitmapImage named m_image. The reason for not making m_image a RefPtr is we do not want to create a cyclic reference: BitmapImage -> ImageSource - > ImageFrameCache -> BitmapImage. But because we allow the decoding thread to continue after the BitmapImage is deleted, we need to ensure the ImageFrameCache does not keep a pointer to the BitmapImage after it is deleted. We can do that by adding a function named ImageFrameCache::clearImage() which will set m_image to null and call it from the BitmapImage destructor via the ImageSource.

This bug can cause the following crash:

WebCore::ImageFrameCache::decodedSizeChanged(long long) + 20 at ImageFrameCache.cpp:127:31
{
       124 	    if (!decodedSize || !m_image || !m_image->imageObserver())
       125 	        return;
       126 	    
    -> 127 	    m_image->imageObserver()->decodedSizeChanged(*m_image, decodedSize);
       128 
}

WebCore::ImageFrameCache::decodedSizeChanged(long long) + 20 at ImageFrameCache.cpp:127:31
WebCore::ImageFrameCache::decodedSizeIncreased(unsigned decodedSize) at ImageFrameCache.cpp:246
WebCore::ImageFrameCache::cacheNativeImageAtIndex(WTF::RetainPtr<CGImage*>&&, unsigned long, WebCore::SubsamplingLevel, WebCore::DecodingOptions const&, WebCore::ImageFrame::DecodingStatus) at ImageFrameCache.cpp:141
WebCore::ImageFrameCache::cacheNativeImageAtIndex(WTF::RetainPtr<CGImage*>&&, unsigned long, WebCore::SubsamplingLevel, WebCore::DecodingOptions const&, WebCore::ImageFrame::DecodingStatus) at ImageFrameCache.cpp:245
WTF::Function<void ()>::CallableWrapper<WebCore::ImageFrameCache::startAsyncDecodingQueue()::$_0::operator()() const::'lambda'()>::call() at ImageFrameCache.cpp:256:5
WTF::Function<void ()>::CallableWrapper<WebCore::ImageFrameCache::startAsyncDecodingQueue()::$_0::operator()() const::'lambda'()>::call() at ImageFrameCache.cpp:305
WTF::Function<void ()>::CallableWrapper<WebCore::ImageFrameCache::startAsyncDecodingQueue()::$_0::operator()() const::'lambda'()>::call() at Function.h:89
Comment 1 Said Abou-Hallawa 2017-05-24 17:30:52 PDT 
<rdar://problem/32385552>
Comment 2 Said Abou-Hallawa 2017-05-24 17:47:08 PDT 
Created attachment 311172 [details]
Patch
Comment 3 Said Abou-Hallawa 2017-05-24 18:34:45 PDT 
Created attachment 311176 [details]
Patch
Comment 4 Said Abou-Hallawa 2017-05-25 10:59:32 PDT 
Created attachment 311247 [details]
Patch
Comment 5 WebKit Commit Bot 2017-05-25 11:22:51 PDT 
Comment on attachment 311247 [details]
Patch

Clearing flags on attachment: 311247

Committed r217437: <http://trac.webkit.org/changeset/217437>
Comment 6 WebKit Commit Bot 2017-05-25 11:22:52 PDT 
All reviewed patches have been landed.  Closing bug.