[oilpan]: Add SafePointAwareMutexLocker to allow GC when waiting for lock.

This change add a mutex locker which enters a safepoint while trying to acquire the mutex lock. This means that threads waiting to acquire a lock would be parked allowing a GC.
In addition to entering a safepoint the locker will also ensure that if we block while leaving the safepoint, due to another thread having scheduled a GC, the lock will be released and the safepoint reentered. This allows for the lock to be acquired during sweeping, specifically in weak processing and finalization which could otherwise deadlock and cause the other scheduled GC to time out.

This change does however not fix the issue where the lock has been acquired and another safepoint is entered while having the lock. This could still cause a timeout. There is an instance of this scenario in the webdatabase's DatabaseBackendBase.cpp::performOpenAndVerify where we get the guidMutex and while commiting the transaction enter a safepoint in the SQLiteStatement::prepare and SQLiteStatement::step calls.
To fix the above we have to restructure the webdatabase code to not hold the global guidMutex lock while doing sync operations.

This change does make it less likely that we hit the empty assert in the terminate-during-sync-websql.html test, but due to the above issue it can still happen. We should consider still doing the restructuring in https://codereview.chromium.org/335823002/ to completely get rid of the flakiness.


R=ager@chromium.org, erik.corry@gmail.com, haraken@chromium.org, oilpan-reviews@chromium.org, tkent@chromium.org, vegorov@chromium.org, zerny@chromium.org
BUG=375671

Review URL: https://codereview.chromium.org/332393002

git-svn-id: svn://svn.chromium.org/blink/trunk@176347 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/modules/webdatabase/DatabaseBackendBase.cpp

@@ -220,7 +220,7 @@ DatabaseBackendBase::DatabaseBackendBase(DatabaseContext* databaseContext, const
         m_name = "";
 
     {
-        MutexLocker locker(guidMutex());
+        SafePointAwareMutexLocker locker(guidMutex());
         m_guid = guidForOriginAndName(securityOrigin()->toString(), name);
         HashSet<DatabaseBackendBase*>* hashSet = guidToDatabaseMap().get(m_guid);
         if (!hashSet) {
@@ -266,7 +266,7 @@ void DatabaseBackendBase::closeDatabase()
     // See comment at the top this file regarding calling removeOpenDatabase().
     DatabaseTracker::tracker().removeOpenDatabase(this);
     {
-        MutexLocker locker(guidMutex());
+        SafePointAwareMutexLocker locker(guidMutex());
 
         HashSet<DatabaseBackendBase*>* hashSet = guidToDatabaseMap().get(m_guid);
         ASSERT(hashSet);
@@ -329,7 +329,7 @@ bool DatabaseBackendBase::performOpenAndVerify(bool shouldSetVersionInNewDatabas
 
     String currentVersion;
     {
-        MutexLocker locker(guidMutex());
+        SafePointAwareMutexLocker locker(guidMutex());
 
         GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
         if (entry != guidToVersionMap().end()) {
@@ -506,14 +506,14 @@ void DatabaseBackendBase::setExpectedVersion(const String& version)
 
 String DatabaseBackendBase::getCachedVersion() const
 {
-    MutexLocker locker(guidMutex());
+    SafePointAwareMutexLocker locker(guidMutex());
     return guidToVersionMap().get(m_guid).isolatedCopy();
 }
 
 void DatabaseBackendBase::setCachedVersion(const String& actualVersion)
 {
     // Update the in memory database version map.
-    MutexLocker locker(guidMutex());
+    SafePointAwareMutexLocker locker(guidMutex());
     updateGuidVersionMap(m_guid, actualVersion);
 }
 

third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteFileSystem.cpp

@@ -31,6 +31,7 @@
 #include "config.h"
 #include "modules/webdatabase/sqlite/SQLiteFileSystem.h"
 
+#include "platform/heap/Handle.h"
 #include <sqlite3.h>
 #include "wtf/text/CString.h"
 
@@ -44,6 +45,7 @@ SQLiteFileSystem::SQLiteFileSystem()
 
 int SQLiteFileSystem::openDatabase(const String& filename, sqlite3** database, bool forWebSQLDatabase)
 {
+    ThreadState::SafePointScope scope(ThreadState::HeapPointersOnStack);
     if (!forWebSQLDatabase)
         return sqlite3_open(filename.utf8().data(), database);
 

third_party/WebKit/Source/modules/webdatabase/sqlite/SQLiteStatement.cpp

@@ -59,12 +59,13 @@ int SQLiteStatement::prepare()
 {
     ASSERT(!m_isPrepared);
 
+    CString query = m_query.stripWhiteSpace().utf8();
+
+    ThreadState::SafePointScope scope(ThreadState::HeapPointersOnStack);
     MutexLocker databaseLock(m_database.databaseMutex());
     if (m_database.isInterrupted())
         return SQLITE_INTERRUPT;
 
-    CString query = m_query.stripWhiteSpace().utf8();
-
     WTF_LOG(SQLDatabase, "SQL - prepare - %s", query.data());
 
     // Pass the length of the string including the null character to sqlite3_prepare_v2;
@@ -87,6 +88,7 @@ int SQLiteStatement::prepare()
 
 int SQLiteStatement::step()
 {
+    ThreadState::SafePointScope scope(ThreadState::HeapPointersOnStack);
     MutexLocker databaseLock(m_database.databaseMutex());
     if (m_database.isInterrupted())
         return SQLITE_INTERRUPT;

third_party/WebKit/Source/platform/heap/ThreadState.cpp

@@ -104,7 +104,7 @@ static Mutex& threadAttachMutex()
 
 static double lockingTimeout()
 {
-    // Wait time for parking all threads is at most 500 MS.
+    // Wait time for parking all threads is at most 100 MS.
     return 0.100;
 }
 
@@ -186,10 +186,17 @@ public:
         ASSERT(ThreadState::current()->isAtSafePoint());
     }
 
-    void checkAndPark(ThreadState* state)
+    void checkAndPark(ThreadState* state, SafePointAwareMutexLocker* locker = 0)
     {
         ASSERT(!state->isSweepInProgress());
         if (!acquireLoad(&m_canResume)) {
+            // If we are leaving the safepoint from a SafePointAwareMutexLocker
+            // call out to release the lock before going to sleep. This enables the
+            // lock to be acquired in the sweep phase, e.g. during weak processing
+            // or finalization. The SafePointAwareLocker will reenter the safepoint
+            // and reacquire the lock after leaving this safepoint.
+            if (locker)
+                locker->reset();
             pushAllRegisters(this, state, parkAfterPushRegisters);
             state->performPendingSweep();
         }
@@ -201,10 +208,10 @@ public:
         pushAllRegisters(this, state, enterSafePointAfterPushRegisters);
     }
 
-    void leaveSafePoint(ThreadState* state)
+    void leaveSafePoint(ThreadState* state, SafePointAwareMutexLocker* locker = 0)
     {
         if (atomicIncrement(&m_unparkedThreadCount) > 0)
-            checkAndPark(state);
+            checkAndPark(state, locker);
     }
 
 private:
@@ -745,8 +752,11 @@ void ThreadState::safePoint(StackState stackState)
 {
     checkThread();
     performPendingGC(stackState);
+    ASSERT(!m_atSafePoint);
     m_stackState = stackState;
+    m_atSafePoint = true;
     s_safePointBarrier->checkAndPark(this);
+    m_atSafePoint = false;
     m_stackState = HeapPointersOnStack;
 }
 
@@ -791,11 +801,11 @@ void ThreadState::enterSafePoint(StackState stackState, void* scopeMarker)
     s_safePointBarrier->enterSafePoint(this);
 }
 
-void ThreadState::leaveSafePoint()
+void ThreadState::leaveSafePoint(SafePointAwareMutexLocker* locker)
 {
     checkThread();
     ASSERT(m_atSafePoint);
-    s_safePointBarrier->leaveSafePoint(this);
+    s_safePointBarrier->leaveSafePoint(this, locker);
     m_atSafePoint = false;
     m_stackState = HeapPointersOnStack;
     clearSafePointScopeMarker();

third_party/WebKit/Source/platform/heap/ThreadState.h

@@ -52,6 +52,7 @@ class HeapObjectHeader;
 class PersistentNode;
 class Visitor;
 class SafePointBarrier;
+class SafePointAwareMutexLocker;
 template<typename Header> class ThreadHeap;
 class CallbackStack;
 
@@ -369,7 +370,7 @@ public:
     // Mark current thread as running inside safepoint.
     void enterSafePointWithoutPointers() { enterSafePoint(NoHeapPointersOnStack, 0); }
     void enterSafePointWithPointers(void* scopeMarker) { enterSafePoint(HeapPointersOnStack, scopeMarker); }
-    void leaveSafePoint();
+    void leaveSafePoint(SafePointAwareMutexLocker* = 0);
     bool isAtSafePoint() const { return m_atSafePoint; }
 
     class SafePointScope {
@@ -510,6 +511,7 @@ private:
     ~ThreadState();
 
     friend class SafePointBarrier;
+    friend class SafePointAwareMutexLocker;
 
     void enterSafePoint(StackState, void*);
     NO_SANITIZE_ADDRESS void copyStackUntilSafePointScope();
@@ -601,6 +603,55 @@ public:
     static ThreadState* state() { return ThreadState::current(); }
 };
 
+// The SafePointAwareMutexLocker is used to enter a safepoint while waiting for
+// a mutex lock. It also ensures that the lock is not held while waiting for a GC
+// to complete in the leaveSafePoint method, by releasing the lock if the
+// leaveSafePoint method cannot complete without blocking, see
+// SafePointBarrier::checkAndPark.
+class SafePointAwareMutexLocker {
+    WTF_MAKE_NONCOPYABLE(SafePointAwareMutexLocker);
+public:
+    explicit SafePointAwareMutexLocker(Mutex& mutex) : m_mutex(mutex), m_locked(false)
+    {
+        ThreadState* state = ThreadState::current();
+        do {
+            bool leaveSafePoint = false;
+            if (!state->isAtSafePoint()) {
+                state->enterSafePoint(ThreadState::HeapPointersOnStack, this);
+                leaveSafePoint = true;
+            }
+            m_mutex.lock();
+            m_locked = true;
+            if (leaveSafePoint) {
+                // When leaving the safepoint we might end up release the mutex
+                // if another thread is requesting a GC, see
+                // SafePointBarrier::checkAndPark. This is the case where we
+                // loop around to reacquire the lock.
+                state->leaveSafePoint(this);
+            }
+        } while (!m_locked);
+    }
+
+    ~SafePointAwareMutexLocker()
+    {
+        ASSERT(m_locked);
+        m_mutex.unlock();
+    }
+
+private:
+    friend class SafePointBarrier;
+
+    void reset()
+    {
+        ASSERT(m_locked);
+        m_mutex.unlock();
+        m_locked = false;
+    }
+
+    Mutex& m_mutex;
+    bool m_locked;
+};
+
 }
 
 #endif // ThreadState_h