Changeset 254169 in webkit

Timestamp:

Jan 7, 2020, 4:17:34 PM (7 years ago)

Author:

sihui_liu@apple.com

Message:

REGRESSION (r248734): different threads write m_storageMap of StorageAreaImpl at the same time
​https://bugs.webkit.org/show_bug.cgi?id=205764
<rdar://problem/58179425>

Reviewed by Maciej Stachowiak.

In StorageAreaImpl, we avoid modifying m_storageMap from different threads at the same time by blocking main
thread access to it until the writes(importing items) of storage thread is done.

In r248734 we introduced a new case where the main thread could modify m_storageMap for session change, but we
didn't add the wait there.

• Storage/StorageAreaImpl.cpp:

(WebKit::StorageAreaImpl::importItems):
(WebKit::StorageAreaImpl::sessionChanged):

Location:

trunk/Source/WebKitLegacy

Files:

• ChangeLog (modified) (1 diff)
• Storage/StorageAreaImpl.cpp (modified) (2 diffs)

trunk/Source/WebKitLegacy/ChangeLog

@@ +1 @@
+2020-01-07  Sihui Liu  <sihui_liu@apple.com>
+
+        REGRESSION (r248734): different threads write m_storageMap of StorageAreaImpl at the same time
+        https://bugs.webkit.org/show_bug.cgi?id=205764
+        <rdar://problem/58179425>
+
+        Reviewed by Maciej Stachowiak.
+
+        In StorageAreaImpl, we avoid modifying m_storageMap from different threads at the same time by blocking main 
+        thread access to it until the writes(importing items) of storage thread is done.
+
+        In r248734 we introduced a new case where the main thread could modify m_storageMap for session change, but we 
+        didn't add the wait there.
+
+        * Storage/StorageAreaImpl.cpp:
+        (WebKit::StorageAreaImpl::importItems):
+        (WebKit::StorageAreaImpl::sessionChanged):
+
 2020-01-06  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/WebKitLegacy/Storage/StorageAreaImpl.cpp

@@ -195 +195 @@
 {
     ASSERT(!m_isShutdown);
+    ASSERT(!isMainThread());
 
     m_storageMap->importItems(WTFMove(items));
@@ -294 +295 @@
     ASSERT(isMainThread());
 
+    // If import is not completed, background storage thread may be modifying m_storageMap.
+    blockUntilImportComplete();
+
     unsigned quota = m_storageMap->quota();
     m_storageMap = StorageMap::create(quota);