[webkit-changes] [WebKit/WebKit] 705360: NetworkStorageManager can be destroyed on wrong th...
Sihui
noreply at github.com
Thu Oct 13 11:17:02 PDT 2022
Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 70536044af3a29b21961944725e332001d67e9ca
https://github.com/WebKit/WebKit/commit/70536044af3a29b21961944725e332001d67e9ca
Author: Sihui Liu <sihui_liu at apple.com>
Date: 2022-10-13 (Thu, 13 Oct 2022)
Changed paths:
M Source/WebKit/NetworkProcess/NetworkProcess.cpp
M Source/WebKit/NetworkProcess/NetworkProcess.h
M Source/WebKit/NetworkProcess/NetworkSession.cpp
M Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp
M Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h
Log Message:
-----------
NetworkStorageManager can be destroyed on wrong thread
https://bugs.webkit.org/show_bug.cgi?id=246375
rdar://101056611
Reviewed by Chris Dumez.
As Kimmo pointed out in bug 245869, NetworkStorageManager can be destroyed on the wrong thread. Here is an example of
the issue:
1. (MainThread) NetworkSession::invalidateAndCancel() is called: NetworkStorageManager::close dispatches a close task to
WorkQueue
2. (MainThread) NetworkProcess::didClose is called: NetworkStorageManager::syncLocalStorage dispatches a task to
WorkQueue
3. (WorkQueue) Task dispatched in step 1 finishes, and a task is dispatched to main thread to release reference of
NetworkStorageManager
4. (MainThread) Main thread task dispatched in step 3 finishes
5. (WorkQueue) Task dispatched in step 2 finishes, and release the last reference of NetworkStorageManager =>
NetworkStorageManager gets destroyed on the wrong thead
The direct cause is syncLocalStorage does not hold strong reference of NetworkStorageManager until completionHandler is
called -- it releases the reference when tasks finishes on background thread. We may fix this by making syncLocalStorage
hold the reference longer, but that's not intuitive, because close() should be the last operation before
NetworkStorageManager gets destroyed. If NetworkStorageManager is closing (close() is called but the tasks have not
finished), we don't need to call syncLocalStorage on it; we just need to wait for the tasks to finish.
To fix this, now we transfer the reference from NetworkSession to NetworkProcess when session is removed and
NetworkStorageManager is closing. This makes sure NetworkProcess has access to all NetworkStorageManagers.
NetworkProcess::didClose only invokes syncLocalStorage on active NetworkStorageManagers, and network process would wait
for closing NetworkStorageManagers to finish before stopping runloop.
* Source/WebKit/NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::stopRunLoopIfNecessary):
(WebKit::NetworkProcess::didClose):
(WebKit::NetworkProcess::destroySession):
* Source/WebKit/NetworkProcess/NetworkProcess.h:
* Source/WebKit/NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::invalidateAndCancel):
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp:
(WebKit::NetworkStorageManager::NetworkStorageManager):
(WebKit::NetworkStorageManager::~NetworkStorageManager):
(WebKit::NetworkStorageManager::close):
(WebKit::NetworkStorageManager::clearStorageForTesting):
(WebKit::NetworkStorageManager::clearStorageForWebPage):
(WebKit::NetworkStorageManager::didIncreaseQuota):
(WebKit::NetworkStorageManager::handleLowMemoryWarning):
(WebKit::NetworkStorageManager::syncLocalStorage):
(WebKit::NetworkStorageManager::registerTemporaryBlobFilePaths):
(WebKit::NetworkStorageManager::requestSpace):
(WebKit::NetworkStorageManager::setBackupExclusionPeriodForTesting):
(WebKit::allNetworkStorageManagers): Deleted.
(WebKit::NetworkStorageManager::forEach): Deleted.
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h:
Canonical link: https://commits.webkit.org/255492@main
More information about the webkit-changes
mailing list