WebKit Bugzilla
Attachment 372295 Details for
Bug 198939
: Protect StorageManager::m_localStorageNamespaces with a Lock
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-198939-20190617170106.patch (text/plain), 6.92 KB, created by
Alex Christensen
on 2019-06-17 17:01:07 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Alex Christensen
Created:
2019-06-17 17:01:07 PDT
Size:
6.92 KB
patch
obsolete
>Index: Source/WebKit/ChangeLog >=================================================================== >--- Source/WebKit/ChangeLog (revision 246522) >+++ Source/WebKit/ChangeLog (working copy) >@@ -1,3 +1,25 @@ >+2019-06-17 Alex Christensen <achristensen@webkit.org> >+ >+ Protect StorageManager::m_localStorageNamespaces with a Lock >+ https://bugs.webkit.org/show_bug.cgi?id=198939 >+ <rdar://problem/51784225> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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 <achristensen@webkit.org> > > Add null check in WebFrameLoaderClient::assignIdentifierToInitialRequest >Index: Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp >=================================================================== >--- Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp (revision 246516) >+++ Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp (working copy) >@@ -376,6 +376,7 @@ void StorageManager::LocalStorageNamespa > 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 @@ void StorageManager::cloneSessionStorage > 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 @@ void StorageManager::getLocalStorageOrig > 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::getLocalStorageOrig > 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 @@ void StorageManager::deleteLocalStorageO > 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 @@ void StorageManager::deleteLocalStorageE > > 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::StorageArea* StorageMana > > StorageManager::LocalStorageNamespace* StorageManager::getOrCreateLocalStorageNamespace(uint64_t storageNamespaceID) > { >+ std::lock_guard<Lock> lock(m_localStorageNamespacesMutex); > if (!m_localStorageNamespaces.isValidKey(storageNamespaceID)) > return nullptr; > >Index: Source/WebKit/NetworkProcess/WebStorage/StorageManager.h >=================================================================== >--- Source/WebKit/NetworkProcess/WebStorage/StorageManager.h (revision 246516) >+++ Source/WebKit/NetworkProcess/WebStorage/StorageManager.h (working copy) >@@ -105,6 +105,7 @@ private: > > 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; >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
ggaren
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 198939
:
372286
| 372295