⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 277343 in webkit


Ignore:
Timestamp:
May 11, 2021, 4:41:22 PM (5 years ago)
Author:
sihui_liu@apple.com
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):

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r277341 r277343  
     12021-05-11  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Use one VM per thread for IDB serialization work
     4        https://bugs.webkit.org/show_bug.cgi?id=225658
     5
     6        Reviewed by Chris Dumez.
     7
     8        The vm map in IDBSerializationContext uses sessionID as key instead of thread identifier. Normally IDB has one
     9        thread per session (see WebIDBServer and CrossThreadTaskHandler), so we are using one vm per thread. With
     10        r275799, we remove WebIDBServer more aggressively (when no web process is not using IDB) to make sure its thread
     11        does not stay around, and WebIDBServer will be destroyed after it finishes scheduled tasks on the background
     12        thread. Then, it's possible that while a WebIDBServer for some session is removed and finishing last tasks,
     13        a new IDB request for the same session comes in and we create a new WebIDBServer for the session. In this case,
     14        two threads ends up using the same VM.
     15
     16        VM is generally not designed to be used on multiple threads, otherwise we need to acquire lock for each
     17        WTF::String operation to get correct AtomStringTable. So let's just make sure we are using one VM per thread by
     18        making the map in IDBSerializationContext keyed by thread pointer.
     19
     20        New API test: IndexedDB.OneVMPerThread
     21
     22        * Modules/indexeddb/server/IDBSerializationContext.cpp:
     23        (WebCore::IDBServer::IDBSerializationContext::getOrCreateIDBSerializationContext):
     24        (WebCore::IDBServer::IDBSerializationContext::~IDBSerializationContext):
     25        (WebCore::IDBServer::IDBSerializationContext::vm):
     26        (WebCore::IDBServer::IDBSerializationContext::globalObject):
     27        (WebCore::IDBServer::IDBSerializationContext::IDBSerializationContext):
     28        * Modules/indexeddb/server/IDBSerializationContext.h:
     29        * Modules/indexeddb/server/MemoryIDBBackingStore.cpp:
     30        (WebCore::IDBServer::MemoryIDBBackingStore::MemoryIDBBackingStore):
     31        * Modules/indexeddb/server/MemoryObjectStore.cpp:
     32        (WebCore::IDBServer::MemoryObjectStore::MemoryObjectStore):
     33        * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
     34        (WebCore::IDBServer::SQLiteIDBBackingStore::SQLiteIDBBackingStore):
     35
    1362021-05-11  Chris Dumez  <cdumez@apple.com>
    237

  • trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.cpp

    r275151 r277343  
    3030#include "WebCoreJSClientData.h"
    3131#include <JavaScriptCore/JSObjectInlines.h>
    32 #include <pal/SessionID.h>
    3332
    3433namespace WebCore {
    … …  
    3837static Lock serializationContextMapMutex;
    3938
    40 static HashMap<PAL::SessionID, IDBSerializationContext*>& serializationContextMap()
     39static HashMap<Thread*, IDBSerializationContext*>& serializationContextMap(Locker<Lock>&)
    4140{
    42     static NeverDestroyed<HashMap<PAL::SessionID, IDBSerializationContext*>> map;
     41    static NeverDestroyed<HashMap<Thread*, IDBSerializationContext*>> map;
    4342    return map;
    4443}
    4544
    46 Ref<IDBSerializationContext> IDBSerializationContext::getOrCreateIDBSerializationContext(PAL::SessionID sessionID)
     45Ref<IDBSerializationContext> IDBSerializationContext::getOrCreateForCurrentThread()
    4746{
     47    auto& thread = Thread::current();
    4848    Locker<Lock> locker(serializationContextMapMutex);
    49     auto[iter, isNewEntry] = serializationContextMap().add(sessionID, nullptr);
     49    auto[iter, isNewEntry] = serializationContextMap(locker).add(&thread, nullptr);
    5050    if (isNewEntry) {
    51         Ref<IDBSerializationContext> protectedContext = adoptRef(*new IDBSerializationContext(sessionID));
     51        Ref<IDBSerializationContext> protectedContext = adoptRef(*new IDBSerializationContext(thread));
    5252        iter->value = protectedContext.ptr();
    5353        return protectedContext;
    … …  
    6060{
    6161    Locker<Lock> locker(serializationContextMapMutex);
    62     ASSERT(this == serializationContextMap().get(m_sessionID));
     62    ASSERT(this == serializationContextMap(locker).get(&m_thread));
    6363
    6464    if (m_vm) {
    … …  
    6767        m_vm = nullptr;
    6868    }
    69     serializationContextMap().remove(m_sessionID);
     69    serializationContextMap(locker).remove(&m_thread);
    7070}
    7171
    … …  
    8686JSC::VM& IDBSerializationContext::vm()
    8787{
     88    ASSERT(&m_thread == &Thread::current());
     89
    8890    initializeVM();
    8991    return *m_vm;
    … …  
    9294JSC::JSGlobalObject& IDBSerializationContext::globalObject()
    9395{
     96    ASSERT(&m_thread == &Thread::current());
     97
    9498    initializeVM();
    9599    return *m_globalObject.get();
    96100}
    97101
    98 IDBSerializationContext::IDBSerializationContext(PAL::SessionID sessionID)
    99     : m_sessionID(sessionID)
     102IDBSerializationContext::IDBSerializationContext(Thread& thread)
     103    : m_thread(thread)
    100104{
    101105}

  • trunk/Source/WebCore/Modules/indexeddb/server/IDBSerializationContext.h

    r275151 r277343  
    2929#include <JavaScriptCore/StrongInlines.h>
    3030#include <JavaScriptCore/StructureInlines.h>
    31 #include <pal/SessionID.h>
    3231
    3332namespace JSC {
    … …  
    4241class IDBSerializationContext : public RefCounted<IDBSerializationContext> {
    4342public:
    44     static Ref<IDBSerializationContext> getOrCreateIDBSerializationContext(PAL::SessionID);
     43    static Ref<IDBSerializationContext> getOrCreateForCurrentThread();
    4544
    4645    ~IDBSerializationContext();
    … …  
    5049
    5150private:
    52     IDBSerializationContext(PAL::SessionID);
     51    explicit IDBSerializationContext(Thread&);
    5352    void initializeVM();
    5453
    5554    RefPtr<JSC::VM> m_vm;
    5655    JSC::Strong<JSIDBSerializationGlobalObject> m_globalObject;
    57     PAL::SessionID m_sessionID;
     56    Thread& m_thread;
    5857};
    5958

  • trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp

    r275583 r277343  
    4949    : m_identifier(identifier)
    5050    , m_sessionID(sessionID)
    51     , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
     51    , m_serializationContext(IDBSerializationContext::getOrCreateForCurrentThread())
    5252{
    5353}

  • trunk/Source/WebCore/Modules/indexeddb/server/MemoryObjectStore.cpp

    r275151 r277343  
    5050}
    5151
    52 MemoryObjectStore::MemoryObjectStore(PAL::SessionID sessionID, const IDBObjectStoreInfo& info)
     52MemoryObjectStore::MemoryObjectStore(PAL::SessionID, const IDBObjectStoreInfo& info)
    5353    : m_info(info)
    54     , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
     54    , m_serializationContext(IDBSerializationContext::getOrCreateForCurrentThread())
    5555{
    5656}

  • trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp

    r277269 r277343  
    248248    , m_identifier(identifier)
    249249    , m_databaseRootDirectory(databaseRootDirectory)
    250     , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
     250    , m_serializationContext(IDBSerializationContext::getOrCreateForCurrentThread())
    251251{
    252252    m_databaseDirectory = fullDatabaseDirectoryWithUpgrade();

  • trunk/Tools/ChangeLog

    r277341 r277343  
     12021-05-11  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Use one VM per thread for IDB serialization work
     4        https://bugs.webkit.org/show_bug.cgi?id=225658
     5
     6        Reviewed by Chris Dumez.
     7
     8        * TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm:
     9        (-[DatabaseProcessKillMessageHandler userContentController:didReceiveScriptMessage:]):
     10        (TEST):
     11
    1122021-05-11  Chris Dumez  <cdumez@apple.com>
    213

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBDatabaseProcessKill.mm

    r275890 r277343  
    2828#import "PlatformUtilities.h"
    2929#import "Test.h"
    30 #import "TestNavigationDelegate.h"
     30#import "TestWKWebView.h"
    3131#import <WebKit/WKProcessPoolPrivate.h>
    3232#import <WebKit/WKUserContentControllerPrivate.h>
    3333#import <WebKit/WKWebViewConfigurationPrivate.h>
     34#import <WebKit/WKWebViewPrivate.h>
    3435#import <WebKit/WKWebsiteDataStorePrivate.h>
    3536#import <WebKit/WebKit.h>
    … …  
    4243static bool openRequestUpgradeNeeded;
    4344static bool databaseErrorReceived;
     45static RetainPtr<NSString> lastScriptMessage;
    4446
    4547@interface DatabaseProcessKillMessageHandler : NSObject <WKScriptMessageHandler>
    … …  
    6769    }
    6870
    69     if ([[message body] isEqualToString:@"OpenRequestError"])
     71    if ([[message body] isEqualToString:@"OpenRequestError"]) {
    7072        receivedAtLeastOneOpenError = true;
     73        return;
     74    }
     75
     76    lastScriptMessage = [message body];
    7177}
    7278
    … …  
    7581TEST(IndexedDB, DatabaseProcessKill)
    7682{
    77     RetainPtr<DatabaseProcessKillMessageHandler> handler = adoptNS([[DatabaseProcessKillMessageHandler alloc] init]);
    78     RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     83    auto handler = adoptNS([[DatabaseProcessKillMessageHandler alloc] init]);
     84    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
    7985    [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"testHandler"];
    8086
    … …  
    101107    EXPECT_EQ(databaseErrorReceived, true);
    102108}
     109
     110TEST(IndexedDB, OneVMPerThread)
     111{
     112    RetainPtr<DatabaseProcessKillMessageHandler> handler = adoptNS([[DatabaseProcessKillMessageHandler alloc] init]);
     113    RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     114    [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"testHandler"];
     115    configuration.get().websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
     116
     117   
     118    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
     119    auto secondWebView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
     120   
     121    NSString *htmlString = @"<script> \
     122        function openDatabase() { \
     123            var request = indexedDB.open('testDB'); \
     124            request.onupgradeneeded = function(event) { \
     125                let db = event.target.result; \
     126                let os = db.createObjectStore('testOS');\
     127                for (let i = 0; i < 10000; i++) \
     128                    os.put(i, i); \
     129                webkit.messageHandlers.testHandler.postMessage('Opened');\
     130            }; \
     131        }\
     132        </script>";
     133
     134    [webView synchronouslyLoadHTMLString:htmlString baseURL:[NSURL URLWithString:@"https://webkit.org"]];
     135    [secondWebView synchronouslyLoadHTMLString:htmlString baseURL:[NSURL URLWithString:@"https://apple.com"]];
     136
     137    receivedScriptMessage = false;
     138    [webView evaluateJavaScript:@"openDatabase()" completionHandler:nil];
     139    TestWebKitAPI::Util::run(&receivedScriptMessage);
     140    EXPECT_WK_STREQ(@"Opened", lastScriptMessage.get());
     141
     142    kill([webView _webProcessIdentifier], SIGKILL);
     143
     144    receivedScriptMessage = false;
     145    [secondWebView evaluateJavaScript:@"openDatabase()" completionHandler:nil];
     146    lastScriptMessage = nil;
     147    TestWebKitAPI::Util::run(&receivedScriptMessage);
     148    EXPECT_WK_STREQ(@"Opened", lastScriptMessage.get());
     149}
Note: See TracChangeset for help on using the changeset viewer.