Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 70536044af3a29b21961944725e332001d67e9ca
https://github.com/WebKit/WebKit/commit/70536044af3a29b21961944725e332001d67e9ca
Author: Sihui Liu <[email protected]>
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
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
