Title: [292426] trunk/Source/WebKit
- Revision
- 292426
- Author
- sihui_...@apple.com
- Date
- 2022-04-05 14:38:05 -0700 (Tue, 05 Apr 2022)
Log Message
Verify generalStorageDirectory is not in use when creating WebsiteDataStore
https://bugs.webkit.org/show_bug.cgi?id=238686
rdar://90808910
Reviewed by Chris Dumez.
In r290739, we added assertion to verify that no two sessions share the same general storage directory. The
assertion is hit on macOS as shown in rdar://90808910. However, because we checked the directories when
launching new network process, not when creating WebsiteDataStore, the backtraces do not give much information
about when the problematic WebsiteDataStore (using the generalStorageDirectory that's already used by another
WebsiteDataStore) is created. To help debug the issue, let's move the assertion to the constructor of
WebsiteDataStore.
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::sendCreationParametersToNewProcess):
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::activeGeneralStorageDirectories):
(WebKit::WebsiteDataStore::WebsiteDataStore):
(WebKit::WebsiteDataStore::~WebsiteDataStore):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (292425 => 292426)
--- trunk/Source/WebKit/ChangeLog 2022-04-05 21:37:35 UTC (rev 292425)
+++ trunk/Source/WebKit/ChangeLog 2022-04-05 21:38:05 UTC (rev 292426)
@@ -1,3 +1,25 @@
+2022-04-05 Sihui Liu <sihui_...@apple.com>
+
+ Verify generalStorageDirectory is not in use when creating WebsiteDataStore
+ https://bugs.webkit.org/show_bug.cgi?id=238686
+ rdar://90808910
+
+ Reviewed by Chris Dumez.
+
+ In r290739, we added assertion to verify that no two sessions share the same general storage directory. The
+ assertion is hit on macOS as shown in rdar://90808910. However, because we checked the directories when
+ launching new network process, not when creating WebsiteDataStore, the backtraces do not give much information
+ about when the problematic WebsiteDataStore (using the generalStorageDirectory that's already used by another
+ WebsiteDataStore) is created. To help debug the issue, let's move the assertion to the constructor of
+ WebsiteDataStore.
+
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::sendCreationParametersToNewProcess):
+ * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+ (WebKit::activeGeneralStorageDirectories):
+ (WebKit::WebsiteDataStore::WebsiteDataStore):
+ (WebKit::WebsiteDataStore::~WebsiteDataStore):
+
2022-04-05 Chris Dumez <cdu...@apple.com>
Unreviewed GTK build fix after r292408.
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (292425 => 292426)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2022-04-05 21:37:35 UTC (rev 292425)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2022-04-05 21:38:05 UTC (rev 292426)
@@ -196,16 +196,7 @@
#if !PLATFORM(GTK) && !PLATFORM(WPE) // GTK and WPE don't use defaultNetworkProcess
parameters.websiteDataStoreParameters = WebsiteDataStore::parametersFromEachWebsiteDataStore();
- HashMap<String, PAL::SessionID> sessionForDirectory;
WebsiteDataStore::forEachWebsiteDataStore([&](auto& websiteDataStore) {
- if (websiteDataStore.isPersistent()) {
- if (auto directory = websiteDataStore.resolvedGeneralStorageDirectory(); !directory.isEmpty()) {
- auto sessionID = websiteDataStore.sessionID();
- // Persistent sessions sharing same storage directory may cause corruption.
- // If the assertion is hit, check if you have multiple WebsiteDataStores with the same generalStorageDirectory.
- RELEASE_ASSERT_WITH_MESSAGE(sessionForDirectory.add(directory, sessionID).isNewEntry, "GeneralStorageDirectory for session %" PRIu64 " is already in use by session %" PRIu64, sessionForDirectory.get(directory).toUInt64(), sessionID.toUInt64());
- }
- }
addSession(websiteDataStore, SendParametersToNetworkProcess::No);
});
#endif
Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (292425 => 292426)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2022-04-05 21:37:35 UTC (rev 292425)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2022-04-05 21:38:05 UTC (rev 292426)
@@ -96,6 +96,12 @@
allowsWebsiteDataRecordsForAllOrigins = true;
}
+static HashMap<String, PAL::SessionID>& activeGeneralStorageDirectories()
+{
+ static MainThreadNeverDestroyed<HashMap<String, PAL::SessionID>> directoryToSessionMap;
+ return directoryToSessionMap;
+}
+
static HashMap<PAL::SessionID, WebsiteDataStore*>& allDataStores()
{
RELEASE_ASSERT(isUIThread());
@@ -139,6 +145,11 @@
platformInitialize();
ASSERT(RunLoop::isMain());
+
+ if (auto directory = m_configuration->generalStorageDirectory(); isPersistent() && !directory.isEmpty()) {
+ if (!activeGeneralStorageDirectories().add(directory, m_sessionID).isNewEntry)
+ RELEASE_LOG_FAULT(Storage, "GeneralStorageDirectory for session %" PRIu64 " is already in use by session %" PRIu64, m_sessionID.toUInt64(), activeGeneralStorageDirectories().get(directory).toUInt64());
+ }
}
WebsiteDataStore::~WebsiteDataStore()
@@ -149,6 +160,8 @@
platformDestroy();
ASSERT(allDataStores().get(m_sessionID) == this);
+ if (auto generalStorageDirectory = m_configuration->generalStorageDirectory(); isPersistent() && !generalStorageDirectory.isEmpty())
+ activeGeneralStorageDirectories().remove(generalStorageDirectory);
allDataStores().remove(m_sessionID);
if (m_networkProcess)
m_networkProcess->removeSession(*this);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
