Commit 9f495748 authored by haraken's avatar haraken Committed by Commit bot

Revert of Enable per thread heap for database thread (patchset #12 id:220001...

Revert of Enable per thread heap for database thread (patchset #12 id:220001 of https://codereview.chromium.org/1909813002/ )

Reason for revert:
This CL is causing a top #1 crasher.

https://bugs.chromium.org/p/chromium/issues/detail?id=617141

Original issue's description:
> Enable per thread heap for database thread
>
> BUG=591606
>
> Committed: https://crrev.com/993e3a74102e0843bebb2f52de1a904f151843d4
> Cr-Commit-Position: refs/heads/master@{#397383}

TBR=oilpan-reviews@chromium.org,keishi@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=591606

Review-Url: https://codereview.chromium.org/2037233002
Cr-Commit-Position: refs/heads/master@{#397884}
parent b3464215
...@@ -215,7 +215,6 @@ Database::Database(DatabaseContext* databaseContext, const String& name, const S ...@@ -215,7 +215,6 @@ Database::Database(DatabaseContext* databaseContext, const String& name, const S
, m_transactionInProgress(false) , m_transactionInProgress(false)
, m_isTransactionQueueEnabled(true) , m_isTransactionQueueEnabled(true)
{ {
DCHECK(isMainThread());
m_contextThreadSecurityOrigin = m_databaseContext->getSecurityOrigin()->isolatedCopy(); m_contextThreadSecurityOrigin = m_databaseContext->getSecurityOrigin()->isolatedCopy();
m_databaseAuthorizer = DatabaseAuthorizer::create(infoTableName); m_databaseAuthorizer = DatabaseAuthorizer::create(infoTableName);
......
...@@ -39,8 +39,6 @@ DatabaseAuthorizer::DatabaseAuthorizer(const String& databaseInfoTableName) ...@@ -39,8 +39,6 @@ DatabaseAuthorizer::DatabaseAuthorizer(const String& databaseInfoTableName)
: m_securityEnabled(false) : m_securityEnabled(false)
, m_databaseInfoTableName(databaseInfoTableName) , m_databaseInfoTableName(databaseInfoTableName)
{ {
DCHECK(isMainThread());
reset(); reset();
addWhitelistedFunctions(); addWhitelistedFunctions();
} }
......
...@@ -100,8 +100,6 @@ DatabaseContext::DatabaseContext(ExecutionContext* context) ...@@ -100,8 +100,6 @@ DatabaseContext::DatabaseContext(ExecutionContext* context)
, m_hasOpenDatabases(false) , m_hasOpenDatabases(false)
, m_hasRequestedTermination(false) , m_hasRequestedTermination(false)
{ {
DCHECK(isMainThread());
// ActiveDOMObject expects this to be called to set internal flags. // ActiveDOMObject expects this to be called to set internal flags.
suspendIfNeeded(); suspendIfNeeded();
......
...@@ -48,7 +48,7 @@ static DatabaseManager* s_databaseManager; ...@@ -48,7 +48,7 @@ static DatabaseManager* s_databaseManager;
DatabaseManager& DatabaseManager::manager() DatabaseManager& DatabaseManager::manager()
{ {
DCHECK(isMainThread()); ASSERT(isMainThread());
if (!s_databaseManager) if (!s_databaseManager)
s_databaseManager = new DatabaseManager(); s_databaseManager = new DatabaseManager();
return *s_databaseManager; return *s_databaseManager;
...@@ -56,7 +56,7 @@ DatabaseManager& DatabaseManager::manager() ...@@ -56,7 +56,7 @@ DatabaseManager& DatabaseManager::manager()
void DatabaseManager::terminateDatabaseThread() void DatabaseManager::terminateDatabaseThread()
{ {
DCHECK(isMainThread()); ASSERT(isMainThread());
if (!s_databaseManager) if (!s_databaseManager)
return; return;
for (const Member<DatabaseContext>& context : s_databaseManager->m_contextMap.values()) for (const Member<DatabaseContext>& context : s_databaseManager->m_contextMap.values())
......
...@@ -41,10 +41,10 @@ namespace blink { ...@@ -41,10 +41,10 @@ namespace blink {
DatabaseThread::DatabaseThread() DatabaseThread::DatabaseThread()
: m_transactionClient(adoptPtr(new SQLTransactionClient())) : m_transactionClient(adoptPtr(new SQLTransactionClient()))
, m_transactionCoordinator(new SQLTransactionCoordinator())
, m_cleanupSync(nullptr) , m_cleanupSync(nullptr)
, m_terminationRequested(false) , m_terminationRequested(false)
{ {
DCHECK(isMainThread());
} }
DatabaseThread::~DatabaseThread() DatabaseThread::~DatabaseThread()
...@@ -55,6 +55,8 @@ DatabaseThread::~DatabaseThread() ...@@ -55,6 +55,8 @@ DatabaseThread::~DatabaseThread()
DEFINE_TRACE(DatabaseThread) DEFINE_TRACE(DatabaseThread)
{ {
visitor->trace(m_openDatabaseSet);
visitor->trace(m_transactionCoordinator);
} }
void DatabaseThread::start() void DatabaseThread::start()
...@@ -62,14 +64,13 @@ void DatabaseThread::start() ...@@ -62,14 +64,13 @@ void DatabaseThread::start()
ASSERT(isMainThread()); ASSERT(isMainThread());
if (m_thread) if (m_thread)
return; return;
m_thread = WebThreadSupportingGC::create("WebCore: Database", true); m_thread = WebThreadSupportingGC::create("WebCore: Database");
m_thread->postTask(BLINK_FROM_HERE, threadSafeBind(&DatabaseThread::setupDatabaseThread, wrapCrossThreadPersistent(this))); m_thread->postTask(BLINK_FROM_HERE, threadSafeBind(&DatabaseThread::setupDatabaseThread, wrapCrossThreadPersistent(this)));
} }
void DatabaseThread::setupDatabaseThread() void DatabaseThread::setupDatabaseThread()
{ {
m_thread->initialize(); m_thread->initialize();
m_transactionCoordinator = new SQLTransactionCoordinator();
} }
void DatabaseThread::terminate() void DatabaseThread::terminate()
...@@ -93,8 +94,6 @@ void DatabaseThread::terminate() ...@@ -93,8 +94,6 @@ void DatabaseThread::terminate()
void DatabaseThread::cleanupDatabaseThread() void DatabaseThread::cleanupDatabaseThread()
{ {
DCHECK(isDatabaseThread());
WTF_LOG(StorageAPI, "Cleaning up DatabaseThread %p", this); WTF_LOG(StorageAPI, "Cleaning up DatabaseThread %p", this);
// Clean up the list of all pending transactions on this database thread // Clean up the list of all pending transactions on this database thread
...@@ -104,10 +103,10 @@ void DatabaseThread::cleanupDatabaseThread() ...@@ -104,10 +103,10 @@ void DatabaseThread::cleanupDatabaseThread()
// inconsistent or locked state. // inconsistent or locked state.
if (m_openDatabaseSet.size() > 0) { if (m_openDatabaseSet.size() > 0) {
// As the call to close will modify the original set, we must take a copy to iterate over. // As the call to close will modify the original set, we must take a copy to iterate over.
HashSet<CrossThreadPersistent<Database>> openSetCopy; HeapHashSet<Member<Database>> openSetCopy;
openSetCopy.swap(m_openDatabaseSet); openSetCopy.swap(m_openDatabaseSet);
HashSet<CrossThreadPersistent<Database>>::iterator end = openSetCopy.end(); HeapHashSet<Member<Database>>::iterator end = openSetCopy.end();
for (HashSet<CrossThreadPersistent<Database>>::iterator it = openSetCopy.begin(); it != end; ++it) for (HeapHashSet<Member<Database>>::iterator it = openSetCopy.begin(); it != end; ++it)
(*it)->close(); (*it)->close();
} }
m_openDatabaseSet.clear(); m_openDatabaseSet.clear();
......
...@@ -77,10 +77,10 @@ private: ...@@ -77,10 +77,10 @@ private:
// This set keeps track of the open databases that have been used on this thread. // This set keeps track of the open databases that have been used on this thread.
// This must be updated in the database thread though it is constructed and // This must be updated in the database thread though it is constructed and
// destructed in the context thread. // destructed in the context thread.
HashSet<CrossThreadPersistent<Database>> m_openDatabaseSet; HeapHashSet<Member<Database>> m_openDatabaseSet;
OwnPtr<SQLTransactionClient> m_transactionClient; OwnPtr<SQLTransactionClient> m_transactionClient;
CrossThreadPersistent<SQLTransactionCoordinator> m_transactionCoordinator; Member<SQLTransactionCoordinator> m_transactionCoordinator;
TaskSynchronizer* m_cleanupSync; TaskSynchronizer* m_cleanupSync;
Mutex m_terminationRequestedMutex; Mutex m_terminationRequestedMutex;
......
...@@ -41,7 +41,6 @@ SQLResultSet::SQLResultSet() ...@@ -41,7 +41,6 @@ SQLResultSet::SQLResultSet()
, m_insertIdSet(false) , m_insertIdSet(false)
, m_isValid(false) , m_isValid(false)
{ {
DCHECK(isMainThread());
} }
DEFINE_TRACE(SQLResultSet) DEFINE_TRACE(SQLResultSet)
......
...@@ -54,8 +54,6 @@ SQLStatement::SQLStatement(Database* database, SQLStatementCallback* callback, ...@@ -54,8 +54,6 @@ SQLStatement::SQLStatement(Database* database, SQLStatementCallback* callback,
: m_statementCallback(callback) : m_statementCallback(callback)
, m_statementErrorCallback(errorCallback) , m_statementErrorCallback(errorCallback)
{ {
DCHECK(isMainThread());
if (hasCallback() || hasErrorCallback()) if (hasCallback() || hasErrorCallback())
InspectorInstrumentation::asyncTaskScheduled(database->getExecutionContext(), "SQLStatement", this); InspectorInstrumentation::asyncTaskScheduled(database->getExecutionContext(), "SQLStatement", this);
} }
......
...@@ -87,8 +87,6 @@ SQLStatementBackend::SQLStatementBackend(SQLStatement* frontend, ...@@ -87,8 +87,6 @@ SQLStatementBackend::SQLStatementBackend(SQLStatement* frontend,
, m_resultSet(SQLResultSet::create()) , m_resultSet(SQLResultSet::create())
, m_permissions(permissions) , m_permissions(permissions)
{ {
DCHECK(isMainThread());
m_frontend->setBackend(this); m_frontend->setBackend(this);
} }
......
...@@ -64,7 +64,6 @@ SQLTransaction::SQLTransaction(Database* db, SQLTransactionCallback* callback, ...@@ -64,7 +64,6 @@ SQLTransaction::SQLTransaction(Database* db, SQLTransactionCallback* callback,
, m_executeSqlAllowed(false) , m_executeSqlAllowed(false)
, m_readOnly(readOnly) , m_readOnly(readOnly)
{ {
DCHECK(isMainThread());
ASSERT(m_database); ASSERT(m_database);
InspectorInstrumentation::asyncTaskScheduled(db->getExecutionContext(), "SQLTransaction", this, true); InspectorInstrumentation::asyncTaskScheduled(db->getExecutionContext(), "SQLTransaction", this, true);
} }
......
...@@ -356,7 +356,6 @@ SQLTransactionBackend::SQLTransactionBackend(Database* db, SQLTransaction* front ...@@ -356,7 +356,6 @@ SQLTransactionBackend::SQLTransactionBackend(Database* db, SQLTransaction* front
, m_readOnly(readOnly) , m_readOnly(readOnly)
, m_hasVersionMismatch(false) , m_hasVersionMismatch(false)
{ {
DCHECK(isMainThread());
ASSERT(m_database); ASSERT(m_database);
m_frontend->setBackend(this); m_frontend->setBackend(this);
m_requestedState = SQLTransactionState::AcquireLock; m_requestedState = SQLTransactionState::AcquireLock;
......
...@@ -50,6 +50,7 @@ SQLTransactionCoordinator::SQLTransactionCoordinator() ...@@ -50,6 +50,7 @@ SQLTransactionCoordinator::SQLTransactionCoordinator()
DEFINE_TRACE(SQLTransactionCoordinator) DEFINE_TRACE(SQLTransactionCoordinator)
{ {
visitor->trace(m_coordinationInfoMap);
} }
void SQLTransactionCoordinator::processPendingTransactions(CoordinationInfo& info) void SQLTransactionCoordinator::processPendingTransactions(CoordinationInfo& info)
...@@ -129,8 +130,11 @@ void SQLTransactionCoordinator::shutdown() ...@@ -129,8 +130,11 @@ void SQLTransactionCoordinator::shutdown()
// transaction is interrupted?" at the top of SQLTransactionBackend.cpp. // transaction is interrupted?" at the top of SQLTransactionBackend.cpp.
if (info.activeWriteTransaction) if (info.activeWriteTransaction)
info.activeWriteTransaction->notifyDatabaseThreadIsShuttingDown(); info.activeWriteTransaction->notifyDatabaseThreadIsShuttingDown();
for (auto& it : info.activeReadTransactions) { for (HeapHashSet<Member<SQLTransactionBackend>>::iterator activeReadTransactionsIterator =
it->notifyDatabaseThreadIsShuttingDown(); info.activeReadTransactions.begin();
activeReadTransactionsIterator != info.activeReadTransactions.end();
++activeReadTransactionsIterator) {
(*activeReadTransactionsIterator)->notifyDatabaseThreadIsShuttingDown();
} }
// Clean up transactions that have NOT reached "lockAcquired": // Clean up transactions that have NOT reached "lockAcquired":
......
...@@ -42,7 +42,7 @@ namespace blink { ...@@ -42,7 +42,7 @@ namespace blink {
class SQLTransactionBackend; class SQLTransactionBackend;
class SQLTransactionCoordinator : public GarbageCollectedFinalized<SQLTransactionCoordinator> { class SQLTransactionCoordinator : public GarbageCollected<SQLTransactionCoordinator> {
WTF_MAKE_NONCOPYABLE(SQLTransactionCoordinator); WTF_MAKE_NONCOPYABLE(SQLTransactionCoordinator);
public: public:
SQLTransactionCoordinator(); SQLTransactionCoordinator();
...@@ -52,16 +52,23 @@ public: ...@@ -52,16 +52,23 @@ public:
void shutdown(); void shutdown();
private: private:
typedef Deque<CrossThreadPersistent<SQLTransactionBackend>> TransactionsQueue; typedef HeapDeque<Member<SQLTransactionBackend>> TransactionsQueue;
struct CoordinationInfo { struct CoordinationInfo {
DISALLOW_NEW_EXCEPT_PLACEMENT_NEW(); DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
public: public:
TransactionsQueue pendingTransactions; TransactionsQueue pendingTransactions;
HashSet<CrossThreadPersistent<SQLTransactionBackend>> activeReadTransactions; HeapHashSet<Member<SQLTransactionBackend>> activeReadTransactions;
CrossThreadPersistent<SQLTransactionBackend> activeWriteTransaction; Member<SQLTransactionBackend> activeWriteTransaction;
DEFINE_INLINE_TRACE()
{
visitor->trace(pendingTransactions);
visitor->trace(activeReadTransactions);
visitor->trace(activeWriteTransaction);
}
}; };
// Maps database names to information about pending transactions // Maps database names to information about pending transactions
typedef HashMap<String, CoordinationInfo> CoordinationInfoHeapMap; typedef HeapHashMap<String, CoordinationInfo> CoordinationInfoHeapMap;
CoordinationInfoHeapMap m_coordinationInfoMap; CoordinationInfoHeapMap m_coordinationInfoMap;
bool m_isShuttingDown; bool m_isShuttingDown;
......
...@@ -205,8 +205,7 @@ public: ...@@ -205,8 +205,7 @@ public:
// TODO(keishi): some tests create CrossThreadPersistent on non attached threads. // TODO(keishi): some tests create CrossThreadPersistent on non attached threads.
if (!ThreadState::current()) if (!ThreadState::current())
return true; return true;
if (&ThreadState::current()->heap() != &pageFromObject(object)->arena()->getThreadState()->heap()) DCHECK(&ThreadState::current()->heap() == &pageFromObject(object)->arena()->getThreadState()->heap());
return true;
return ObjectAliveTrait<T>::isHeapObjectAlive(object); return ObjectAliveTrait<T>::isHeapObjectAlive(object);
} }
template<typename T> template<typename T>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment