- Revision
- 277343
- Author
- [email protected]
- Date
- 2021-05-11 16:41:22 -0700 (Tue, 11 May 2021)
Log Message
Use one VM per thread for IDB serialization work
https://bugs.webkit.org/show_bug.cgi?id=225658
Reviewed by Chris Dumez.
Source/WebCore:
The vm map in IDBSerializationContext uses sessionID as key instead of thread identifier. Normally IDB has one
thread per session (see WebIDBServer and CrossThreadTaskHandler), so we are using one vm per thread. With
r275799, we remove WebIDBServer more aggressively (when no web process is not using IDB) to make sure its thread
does not stay around, and WebIDBServer will be destroyed after it finishes scheduled tasks on the background
thread. Then, it's possible that while a WebIDBServer for some session is removed and finishing last tasks,
a new IDB request for the same session comes in and we create a new WebIDBServer for the session. In this case,
two threads ends up using the same VM.
VM is generally not designed to be used on multiple threads, otherwise we need to acquire lock for each
WTF::String operation to get correct AtomStringTable. So let's just make sure we are using one VM per thread by
making the map in IDBSerializationContext keyed by thread pointer.
New API test: IndexedDB.OneVMPerThread
* Modules/indexeddb/server/IDBSerializationContext.cpp:
(WebCore::IDBServer::IDBSerializationContext::getOrCreateIDBSerializationContext):
(WebCore::IDBServer::IDBSerializationContext::~IDBSerializationContext):
(WebCore::IDBServer::IDBSerializationContext::vm):
(WebCore::IDBServer::IDBSerializationContext::globalObject):
(WebCore::IDBServer::IDBSerializationContext::IDBSerializationContext):
* Modules/indexeddb/server/IDBSerializationContext.h:
* Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
(WebCore::IDBServer::MemoryIDBBackingStore::MemoryIDBBackingStore):
* Modules/indexeddb/server/MemoryObjectStore.cpp:
(WebCore::IDBServer::MemoryObjectStore::MemoryObjectStore):
* Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
(WebCore::IDBServer::SQLiteIDBBackingStore::SQLiteIDBBackingStore):
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm:
(-[DatabaseProcessKillMessageHandler userContentController:didReceiveScriptMessage:]):
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (277342 => 277343)
--- trunk/Source/WebCore/ChangeLog 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Source/WebCore/ChangeLog 2021-05-11 23:41:22 UTC (rev 277343)
@@ -1,3 +1,38 @@
+2021-05-11 Sihui Liu <[email protected]>
+
+ Use one VM per thread for IDB serialization work
+ https://bugs.webkit.org/show_bug.cgi?id=225658
+
+ Reviewed by Chris Dumez.
+
+ The vm map in IDBSerializationContext uses sessionID as key instead of thread identifier. Normally IDB has one
+ thread per session (see WebIDBServer and CrossThreadTaskHandler), so we are using one vm per thread. With
+ r275799, we remove WebIDBServer more aggressively (when no web process is not using IDB) to make sure its thread
+ does not stay around, and WebIDBServer will be destroyed after it finishes scheduled tasks on the background
+ thread. Then, it's possible that while a WebIDBServer for some session is removed and finishing last tasks,
+ a new IDB request for the same session comes in and we create a new WebIDBServer for the session. In this case,
+ two threads ends up using the same VM.
+
+ VM is generally not designed to be used on multiple threads, otherwise we need to acquire lock for each
+ WTF::String operation to get correct AtomStringTable. So let's just make sure we are using one VM per thread by
+ making the map in IDBSerializationContext keyed by thread pointer.
+
+ New API test: IndexedDB.OneVMPerThread
+
+ * Modules/indexeddb/server/IDBSerializationContext.cpp:
+ (WebCore::IDBServer::IDBSerializationContext::getOrCreateIDBSerializationContext):
+ (WebCore::IDBServer::IDBSerializationContext::~IDBSerializationContext):
+ (WebCore::IDBServer::IDBSerializationContext::vm):
+ (WebCore::IDBServer::IDBSerializationContext::globalObject):
+ (WebCore::IDBServer::IDBSerializationContext::IDBSerializationContext):
+ * Modules/indexeddb/server/IDBSerializationContext.h:
+ * Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
+ (WebCore::IDBServer::MemoryIDBBackingStore::MemoryIDBBackingStore):
+ * Modules/indexeddb/server/MemoryObjectStore.cpp:
+ (WebCore::IDBServer::MemoryObjectStore::MemoryObjectStore):
+ * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
+ (WebCore::IDBServer::SQLiteIDBBackingStore::SQLiteIDBBackingStore):
+
2021-05-11 Chris Dumez <[email protected]>
Add SPI to suspend / resume a WKWebView
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.cpp (277342 => 277343)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.cpp 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.cpp 2021-05-11 23:41:22 UTC (rev 277343)
@@ -29,7 +29,6 @@
#include "DOMWrapperWorld.h"
#include "WebCoreJSClientData.h"
#include <_javascript_Core/JSObjectInlines.h>
-#include <pal/SessionID.h>
namespace WebCore {
@@ -37,18 +36,19 @@
static Lock serializationContextMapMutex;
-static HashMap<PAL::SessionID, IDBSerializationContext*>& serializationContextMap()
+static HashMap<Thread*, IDBSerializationContext*>& serializationContextMap(Locker<Lock>&)
{
- static NeverDestroyed<HashMap<PAL::SessionID, IDBSerializationContext*>> map;
+ static NeverDestroyed<HashMap<Thread*, IDBSerializationContext*>> map;
return map;
}
-Ref<IDBSerializationContext> IDBSerializationContext::getOrCreateIDBSerializationContext(PAL::SessionID sessionID)
+Ref<IDBSerializationContext> IDBSerializationContext::getOrCreateForCurrentThread()
{
+ auto& thread = Thread::current();
Locker<Lock> locker(serializationContextMapMutex);
- auto[iter, isNewEntry] = serializationContextMap().add(sessionID, nullptr);
+ auto[iter, isNewEntry] = serializationContextMap(locker).add(&thread, nullptr);
if (isNewEntry) {
- Ref<IDBSerializationContext> protectedContext = adoptRef(*new IDBSerializationContext(sessionID));
+ Ref<IDBSerializationContext> protectedContext = adoptRef(*new IDBSerializationContext(thread));
iter->value = protectedContext.ptr();
return protectedContext;
}
@@ -59,7 +59,7 @@
IDBSerializationContext::~IDBSerializationContext()
{
Locker<Lock> locker(serializationContextMapMutex);
- ASSERT(this == serializationContextMap().get(m_sessionID));
+ ASSERT(this == serializationContextMap(locker).get(&m_thread));
if (m_vm) {
JSC::JSLockHolder lock(*m_vm);
@@ -66,7 +66,7 @@
m_globalObject.clear();
m_vm = nullptr;
}
- serializationContextMap().remove(m_sessionID);
+ serializationContextMap(locker).remove(&m_thread);
}
void IDBSerializationContext::initializeVM()
@@ -85,6 +85,8 @@
JSC::VM& IDBSerializationContext::vm()
{
+ ASSERT(&m_thread == &Thread::current());
+
initializeVM();
return *m_vm;
}
@@ -91,12 +93,14 @@
JSC::JSGlobalObject& IDBSerializationContext::globalObject()
{
+ ASSERT(&m_thread == &Thread::current());
+
initializeVM();
return *m_globalObject.get();
}
-IDBSerializationContext::IDBSerializationContext(PAL::SessionID sessionID)
- : m_sessionID(sessionID)
+IDBSerializationContext::IDBSerializationContext(Thread& thread)
+ : m_thread(thread)
{
}
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.h (277342 => 277343)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.h 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.h 2021-05-11 23:41:22 UTC (rev 277343)
@@ -28,7 +28,6 @@
#include "JSIDBSerializationGlobalObject.h"
#include <_javascript_Core/StrongInlines.h>
#include <_javascript_Core/StructureInlines.h>
-#include <pal/SessionID.h>
namespace JSC {
class CallFrame;
@@ -41,7 +40,7 @@
class IDBSerializationContext : public RefCounted<IDBSerializationContext> {
public:
- static Ref<IDBSerializationContext> getOrCreateIDBSerializationContext(PAL::SessionID);
+ static Ref<IDBSerializationContext> getOrCreateForCurrentThread();
~IDBSerializationContext();
@@ -49,12 +48,12 @@
JSC::JSGlobalObject& globalObject();
private:
- IDBSerializationContext(PAL::SessionID);
+ explicit IDBSerializationContext(Thread&);
void initializeVM();
RefPtr<JSC::VM> m_vm;
JSC::Strong<JSIDBSerializationGlobalObject> m_globalObject;
- PAL::SessionID m_sessionID;
+ Thread& m_thread;
};
} // namespace IDBServer
Modified: trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp (277342 => 277343)
--- trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp 2021-05-11 23:41:22 UTC (rev 277343)
@@ -48,7 +48,7 @@
MemoryIDBBackingStore::MemoryIDBBackingStore(PAL::SessionID sessionID, const IDBDatabaseIdentifier& identifier)
: m_identifier(identifier)
, m_sessionID(sessionID)
- , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
+ , m_serializationContext(IDBSerializationContext::getOrCreateForCurrentThread())
{
}
Modified: trunk/Source/WebCore/Modules/indexeddb/server/MemoryObjectStore.cpp (277342 => 277343)
--- trunk/Source/WebCore/Modules/indexeddb/server/MemoryObjectStore.cpp 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Source/WebCore/Modules/indexeddb/server/MemoryObjectStore.cpp 2021-05-11 23:41:22 UTC (rev 277343)
@@ -49,9 +49,9 @@
return adoptRef(*new MemoryObjectStore(sessionID, info));
}
-MemoryObjectStore::MemoryObjectStore(PAL::SessionID sessionID, const IDBObjectStoreInfo& info)
+MemoryObjectStore::MemoryObjectStore(PAL::SessionID, const IDBObjectStoreInfo& info)
: m_info(info)
- , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
+ , m_serializationContext(IDBSerializationContext::getOrCreateForCurrentThread())
{
}
Modified: trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp (277342 => 277343)
--- trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp 2021-05-11 23:41:22 UTC (rev 277343)
@@ -247,7 +247,7 @@
: m_sessionID(sessionID)
, m_identifier(identifier)
, m_databaseRootDirectory(databaseRootDirectory)
- , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
+ , m_serializationContext(IDBSerializationContext::getOrCreateForCurrentThread())
{
m_databaseDirectory = fullDatabaseDirectoryWithUpgrade();
}
Modified: trunk/Tools/ChangeLog (277342 => 277343)
--- trunk/Tools/ChangeLog 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Tools/ChangeLog 2021-05-11 23:41:22 UTC (rev 277343)
@@ -1,3 +1,14 @@
+2021-05-11 Sihui Liu <[email protected]>
+
+ Use one VM per thread for IDB serialization work
+ https://bugs.webkit.org/show_bug.cgi?id=225658
+
+ Reviewed by Chris Dumez.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm:
+ (-[DatabaseProcessKillMessageHandler userContentController:didReceiveScriptMessage:]):
+ (TEST):
+
2021-05-11 Chris Dumez <[email protected]>
Add SPI to suspend / resume a WKWebView
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm (277342 => 277343)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm 2021-05-11 23:30:19 UTC (rev 277342)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm 2021-05-11 23:41:22 UTC (rev 277343)
@@ -27,10 +27,11 @@
#import "PlatformUtilities.h"
#import "Test.h"
-#import "TestNavigationDelegate.h"
+#import "TestWKWebView.h"
#import <WebKit/WKProcessPoolPrivate.h>
#import <WebKit/WKUserContentControllerPrivate.h>
#import <WebKit/WKWebViewConfigurationPrivate.h>
+#import <WebKit/WKWebViewPrivate.h>
#import <WebKit/WKWebsiteDataStorePrivate.h>
#import <WebKit/WebKit.h>
#import <WebKit/_WKProcessPoolConfiguration.h>
@@ -41,6 +42,7 @@
static bool receivedAtLeastOneDeleteError;
static bool openRequestUpgradeNeeded;
static bool databaseErrorReceived;
+static RetainPtr<NSString> lastScriptMessage;
@interface DatabaseProcessKillMessageHandler : NSObject <WKScriptMessageHandler>
@end
@@ -66,8 +68,12 @@
return;
}
- if ([[message body] isEqualToString:@"OpenRequestError"])
+ if ([[message body] isEqualToString:@"OpenRequestError"]) {
receivedAtLeastOneOpenError = true;
+ return;
+ }
+
+ lastScriptMessage = [message body];
}
@end
@@ -74,8 +80,8 @@
TEST(IndexedDB, DatabaseProcessKill)
{
- RetainPtr<DatabaseProcessKillMessageHandler> handler = adoptNS([[DatabaseProcessKillMessageHandler alloc] init]);
- RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto handler = adoptNS([[DatabaseProcessKillMessageHandler alloc] init]);
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
[[configuration userContentController] addScriptMessageHandler:handler.get() name:@"testHandler"];
RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
@@ -100,3 +106,44 @@
EXPECT_EQ(receivedAtLeastOneDeleteError, true);
EXPECT_EQ(databaseErrorReceived, true);
}
+
+TEST(IndexedDB, OneVMPerThread)
+{
+ RetainPtr<DatabaseProcessKillMessageHandler> handler = adoptNS([[DatabaseProcessKillMessageHandler alloc] init]);
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"testHandler"];
+ configuration.get().websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
+
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
+ auto secondWebView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
+
+ NSString *htmlString = @"<script> \
+ function openDatabase() { \
+ var request = indexedDB.open('testDB'); \
+ request._onupgradeneeded_ = function(event) { \
+ let db = event.target.result; \
+ let os = db.createObjectStore('testOS');\
+ for (let i = 0; i < 10000; i++) \
+ os.put(i, i); \
+ webkit.messageHandlers.testHandler.postMessage('Opened');\
+ }; \
+ }\
+ </script>";
+
+ [webView synchronouslyLoadHTMLString:htmlString baseURL:[NSURL URLWithString:@"https://webkit.org"]];
+ [secondWebView synchronouslyLoadHTMLString:htmlString baseURL:[NSURL URLWithString:@"https://apple.com"]];
+
+ receivedScriptMessage = false;
+ [webView evaluateJavaScript:@"openDatabase()" completionHandler:nil];
+ TestWebKitAPI::Util::run(&receivedScriptMessage);
+ EXPECT_WK_STREQ(@"Opened", lastScriptMessage.get());
+
+ kill([webView _webProcessIdentifier], SIGKILL);
+
+ receivedScriptMessage = false;
+ [secondWebView evaluateJavaScript:@"openDatabase()" completionHandler:nil];
+ lastScriptMessage = nil;
+ TestWebKitAPI::Util::run(&receivedScriptMessage);
+ EXPECT_WK_STREQ(@"Opened", lastScriptMessage.get());
+}
