Changeset 234020 in webkit


Ignore:
Timestamp:
Jul 19, 2018 8:44:58 PM (6 years ago)
Author:
Chris Dumez
Message:

[ITP] Crash under ResourceLoadStatisticsMemoryStore::removeDataRecords()
https://bugs.webkit.org/show_bug.cgi?id=187821
<rdar://problem/42112693>

Reviewed by David Kilzer.

In two cases, ResourceLoadStatisticsMemoryStore (which lives on a background queue) needs to call WebPageProxy
operations on the main thread and then dispatch back on the background queue when the operation completes.
However, it is possible for the ResourceLoadStatisticsMemoryStore to get destroyed on the background queue
during this time and we would then crash when trying to use m_workQueue to re-dispatch. To address the issue,
I now ref the work queue in the lambda so that we're guaranteed to be able to re-dispatch to the background
queue. When we're back on the background queue, we'll realize that weakThis in gone and we'll call the callback
and return early.

Note that I am not checking weakThis on the main thread as this would not be safe. weakThis should only be
used on the background queue.

  • UIProcess/ResourceLoadStatisticsMemoryStore.cpp:

(WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords):
(WebKit::ResourceLoadStatisticsMemoryStore::grandfatherExistingWebsiteData):

Location:
trunk/Source/WebKit
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r234019 r234020  
     12018-07-19  Chris Dumez  <cdumez@apple.com>
     2
     3        [ITP] Crash under ResourceLoadStatisticsMemoryStore::removeDataRecords()
     4        https://bugs.webkit.org/show_bug.cgi?id=187821
     5        <rdar://problem/42112693>
     6
     7        Reviewed by David Kilzer.
     8
     9        In two cases, ResourceLoadStatisticsMemoryStore (which lives on a background queue) needs to call WebPageProxy
     10        operations on the main thread and then dispatch back on the background queue when the operation completes.
     11        However, it is possible for the ResourceLoadStatisticsMemoryStore to get destroyed on the background queue
     12        during this time and we would then crash when trying to use m_workQueue to re-dispatch. To address the issue,
     13        I now ref the work queue in the lambda so that we're guaranteed to be able to re-dispatch to the background
     14        queue. When we're back on the background queue, we'll realize that weakThis in gone and we'll call the callback
     15        and return early.
     16
     17        Note that I am not checking weakThis on the main thread as this would not be safe. weakThis should only be
     18        used on the background queue.
     19
     20        * UIProcess/ResourceLoadStatisticsMemoryStore.cpp:
     21        (WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords):
     22        (WebKit::ResourceLoadStatisticsMemoryStore::grandfatherExistingWebsiteData):
     23
    1242018-07-19  Fujii Hironori  <Hironori.Fujii@sony.com>
    225
  • trunk/Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp

    r233381 r234020  
    246246
    247247    RunLoop::main().dispatch([prevalentResourceDomains = crossThreadCopy(prevalentResourceDomains), callback = WTFMove(callback), this, weakThis = makeWeakPtr(*this)] () mutable {
    248         WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(prevalentResourceDomains), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [callback = WTFMove(callback), this, weakThis = WTFMove(weakThis)](const HashSet<String>& domainsWithDeletedWebsiteData) mutable {
    249             m_workQueue->dispatch([topDomains = crossThreadCopy(domainsWithDeletedWebsiteData), callback = WTFMove(callback), this, weakThis = WTFMove(weakThis)] () mutable {
     248        WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(prevalentResourceDomains), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [callback = WTFMove(callback), this, weakThis = WTFMove(weakThis), workQueue = m_workQueue.copyRef()](const HashSet<String>& domainsWithDeletedWebsiteData) mutable {
     249            workQueue->dispatch([topDomains = crossThreadCopy(domainsWithDeletedWebsiteData), callback = WTFMove(callback), this, weakThis = WTFMove(weakThis)] () mutable {
    250250                if (!weakThis) {
    251251                    callback();
     
    487487        // FIXME: This method being a static call on WebProcessProxy is wrong.
    488488        // It should be on the data store that this object belongs to.
    489         WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData(WebResourceLoadStatisticsStore::monitoredDataTypes(), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [this, weakThis = WTFMove(weakThis), callback = WTFMove(callback)] (HashSet<String>&& topPrivatelyControlledDomainsWithWebsiteData) mutable {
    490             m_workQueue->dispatch([this, weakThis = WTFMove(weakThis), topDomains = crossThreadCopy(topPrivatelyControlledDomainsWithWebsiteData), callback = WTFMove(callback)] () mutable {
     489        WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData(WebResourceLoadStatisticsStore::monitoredDataTypes(), m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, [this, weakThis = WTFMove(weakThis), callback = WTFMove(callback), workQueue = m_workQueue.copyRef()] (HashSet<String>&& topPrivatelyControlledDomainsWithWebsiteData) mutable {
     490            workQueue->dispatch([this, weakThis = WTFMove(weakThis), topDomains = crossThreadCopy(topPrivatelyControlledDomainsWithWebsiteData), callback = WTFMove(callback)] () mutable {
    491491                if (!weakThis) {
    492492                    callback();
Note: See TracChangeset for help on using the changeset viewer.