Title: [246526] trunk/Source/WebKit
- Revision
- 246526
- Author
- [email protected]
- Date
- 2019-06-17 17:17:24 -0700 (Mon, 17 Jun 2019)
Log Message
Protect StorageManager::m_localStorageNamespaces with a Lock
https://bugs.webkit.org/show_bug.cgi?id=198939
<rdar://problem/51784225>
Reviewed by Geoff Garen.
StorageManager::LocalStorageNamespace::didDestroyStorageArea is called from StorageArea::~StorageArea which is called on the main thread.
All other access of m_localStorageNamespaces is from the non-main thread. Sometimes this causes hash table corruption, so wait for a mutex
before accessing this member variable.
* NetworkProcess/WebStorage/StorageManager.cpp:
(WebKit::StorageManager::LocalStorageNamespace::didDestroyStorageArea):
(WebKit::StorageManager::cloneSessionStorageNamespace):
(WebKit::StorageManager::getLocalStorageOrigins):
(WebKit::StorageManager::deleteLocalStorageEntriesForOrigin):
(WebKit::StorageManager::deleteLocalStorageOriginsModifiedSince):
(WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
(WebKit::StorageManager::getOrCreateLocalStorageNamespace):
* NetworkProcess/WebStorage/StorageManager.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (246525 => 246526)
--- trunk/Source/WebKit/ChangeLog 2019-06-18 00:14:59 UTC (rev 246525)
+++ trunk/Source/WebKit/ChangeLog 2019-06-18 00:17:24 UTC (rev 246526)
@@ -1,5 +1,27 @@
2019-06-17 Alex Christensen <[email protected]>
+ Protect StorageManager::m_localStorageNamespaces with a Lock
+ https://bugs.webkit.org/show_bug.cgi?id=198939
+ <rdar://problem/51784225>
+
+ Reviewed by Geoff Garen.
+
+ StorageManager::LocalStorageNamespace::didDestroyStorageArea is called from StorageArea::~StorageArea which is called on the main thread.
+ All other access of m_localStorageNamespaces is from the non-main thread. Sometimes this causes hash table corruption, so wait for a mutex
+ before accessing this member variable.
+
+ * NetworkProcess/WebStorage/StorageManager.cpp:
+ (WebKit::StorageManager::LocalStorageNamespace::didDestroyStorageArea):
+ (WebKit::StorageManager::cloneSessionStorageNamespace):
+ (WebKit::StorageManager::getLocalStorageOrigins):
+ (WebKit::StorageManager::deleteLocalStorageEntriesForOrigin):
+ (WebKit::StorageManager::deleteLocalStorageOriginsModifiedSince):
+ (WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
+ (WebKit::StorageManager::getOrCreateLocalStorageNamespace):
+ * NetworkProcess/WebStorage/StorageManager.h:
+
+2019-06-17 Alex Christensen <[email protected]>
+
Add null check in WebFrameLoaderClient::assignIdentifierToInitialRequest
https://bugs.webkit.org/show_bug.cgi?id=198926
<rdar://problem/50079713>
Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp (246525 => 246526)
--- trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp 2019-06-18 00:14:59 UTC (rev 246525)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp 2019-06-18 00:17:24 UTC (rev 246526)
@@ -376,6 +376,7 @@
if (!m_storageAreaMap.isEmpty())
return;
+ std::lock_guard<Lock> lock(m_storageManager.m_localStorageNamespacesMutex);
ASSERT(m_storageManager.m_localStorageNamespaces.contains(m_storageNamespaceID));
m_storageManager.m_localStorageNamespaces.remove(m_storageNamespaceID);
}
@@ -572,6 +573,7 @@
sessionStorageNamespace->cloneTo(*newSessionStorageNamespace);
if (!m_localStorageDatabaseTracker) {
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
if (auto* localStorageNamespace = m_localStorageNamespaces.get(storageNamespaceID)) {
LocalStorageNamespace* newlocalStorageNamespace = getOrCreateLocalStorageNamespace(newStorageNamespaceID);
localStorageNamespace->cloneTo(*newlocalStorageNamespace);
@@ -662,6 +664,7 @@
for (auto& origin : m_localStorageDatabaseTracker->origins())
origins.add(origin);
} else {
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
for (const auto& localStorageNameSpace : m_localStorageNamespaces.values()) {
for (auto& origin : localStorageNameSpace->ephemeralOrigins())
origins.add(origin);
@@ -695,8 +698,11 @@
void StorageManager::deleteLocalStorageEntriesForOrigin(const SecurityOriginData& securityOrigin)
{
m_queue->dispatch([this, protectedThis = makeRef(*this), copiedOrigin = securityOrigin.isolatedCopy()]() mutable {
- for (auto& localStorageNamespace : m_localStorageNamespaces.values())
- localStorageNamespace->clearStorageAreasMatchingOrigin(copiedOrigin);
+ {
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
+ for (auto& localStorageNamespace : m_localStorageNamespaces.values())
+ localStorageNamespace->clearStorageAreasMatchingOrigin(copiedOrigin);
+ }
for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
transientLocalStorageNamespace->clearStorageAreasMatchingOrigin(copiedOrigin);
@@ -716,12 +722,16 @@
transientLocalStorageNamespace->clearAllStorageAreas();
for (const auto& origin : originsToDelete) {
- for (auto& localStorageNamespace : m_localStorageNamespaces.values())
- localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
+ {
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
+ for (auto& localStorageNamespace : m_localStorageNamespaces.values())
+ localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
+ }
m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin);
}
} else {
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
for (auto& localStorageNamespace : m_localStorageNamespaces.values())
localStorageNamespace->clearAllStorageAreas();
}
@@ -740,8 +750,11 @@
m_queue->dispatch([this, protectedThis = makeRef(*this), copiedOrigins = WTFMove(copiedOrigins), completionHandler = WTFMove(completionHandler)]() mutable {
for (auto& origin : copiedOrigins) {
- for (auto& localStorageNamespace : m_localStorageNamespaces.values())
- localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
+ {
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
+ for (auto& localStorageNamespace : m_localStorageNamespaces.values())
+ localStorageNamespace->clearStorageAreasMatchingOrigin(origin);
+ }
for (auto& transientLocalStorageNamespace : m_transientLocalStorageNamespaces.values())
transientLocalStorageNamespace->clearStorageAreasMatchingOrigin(origin);
@@ -999,6 +1012,7 @@
StorageManager::LocalStorageNamespace* StorageManager::getOrCreateLocalStorageNamespace(uint64_t storageNamespaceID)
{
+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex);
if (!m_localStorageNamespaces.isValidKey(storageNamespaceID))
return nullptr;
Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h (246525 => 246526)
--- trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h 2019-06-18 00:14:59 UTC (rev 246525)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h 2019-06-18 00:17:24 UTC (rev 246526)
@@ -105,6 +105,7 @@
RefPtr<LocalStorageDatabaseTracker> m_localStorageDatabaseTracker;
HashMap<uint64_t, RefPtr<LocalStorageNamespace>> m_localStorageNamespaces;
+ Lock m_localStorageNamespacesMutex;
HashMap<std::pair<uint64_t, WebCore::SecurityOriginData>, RefPtr<TransientLocalStorageNamespace>> m_transientLocalStorageNamespaces;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes