Diff
Modified: trunk/Source/WTF/ChangeLog (277921 => 277922)
--- trunk/Source/WTF/ChangeLog 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WTF/ChangeLog 2021-05-22 21:40:04 UTC (rev 277922)
@@ -1,5 +1,20 @@
2021-05-22 Chris Dumez <cdu...@apple.com>
+ Adopt CheckedLock in more places
+ https://bugs.webkit.org/show_bug.cgi?id=226138
+
+ Reviewed by Darin Adler.
+
+ Adopt CheckedLock in more places to benefit from Clang Thread Safety Analysis.
+
+ * wtf/Assertions.cpp:
+ * wtf/TimingScope.cpp:
+ * wtf/threads/BinarySemaphore.cpp:
+ (WTF::BinarySemaphore::waitUntil):
+ * wtf/threads/BinarySemaphore.h:
+
+2021-05-22 Chris Dumez <cdu...@apple.com>
+
Replace LockHolder with Locker in local variables
https://bugs.webkit.org/show_bug.cgi?id=226133
Modified: trunk/Source/WTF/wtf/Assertions.cpp (277921 => 277922)
--- trunk/Source/WTF/wtf/Assertions.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WTF/wtf/Assertions.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -31,8 +31,8 @@
#include <mutex>
#include <stdio.h>
#include <string.h>
+#include <wtf/CheckedLock.h>
#include <wtf/Compiler.h>
-#include <wtf/Lock.h>
#include <wtf/Locker.h>
#include <wtf/LoggingAccumulator.h>
#include <wtf/PrintStream.h>
@@ -368,25 +368,25 @@
String getAndResetAccumulatedLogs();
private:
- Lock accumulatorLock;
- StringBuilder loggingAccumulator;
+ CheckedLock accumulatorLock;
+ StringBuilder loggingAccumulator WTF_GUARDED_BY_LOCK(accumulatorLock);
};
void WTFLoggingAccumulator::accumulate(const String& log)
{
- Locker<Lock> locker(accumulatorLock);
+ Locker locker { accumulatorLock };
loggingAccumulator.append(log);
}
void WTFLoggingAccumulator::resetAccumulatedLogs()
{
- Locker<Lock> locker(accumulatorLock);
+ Locker locker { accumulatorLock };
loggingAccumulator.clear();
}
String WTFLoggingAccumulator::getAndResetAccumulatedLogs()
{
- Locker<Lock> locker(accumulatorLock);
+ Locker locker { accumulatorLock };
String result = loggingAccumulator.toString();
loggingAccumulator.clear();
return result;
Modified: trunk/Source/WTF/wtf/TimingScope.cpp (277921 => 277922)
--- trunk/Source/WTF/wtf/TimingScope.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WTF/wtf/TimingScope.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -26,8 +26,8 @@
#include "config.h"
#include <wtf/TimingScope.h>
+#include <wtf/CheckedLock.h>
#include <wtf/HashMap.h>
-#include <wtf/Lock.h>
namespace WTF {
@@ -59,8 +59,8 @@
}
private:
- HashMap<const char*, CallData> totals;
- Lock lock;
+ HashMap<const char*, CallData> totals WTF_GUARDED_BY_LOCK(lock);
+ CheckedLock lock;
};
State& state()
Modified: trunk/Source/WTF/wtf/threads/BinarySemaphore.cpp (277921 => 277922)
--- trunk/Source/WTF/wtf/threads/BinarySemaphore.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WTF/wtf/threads/BinarySemaphore.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -38,7 +38,10 @@
bool BinarySemaphore::waitUntil(const TimeWithDynamicClockType& absoluteTime)
{
Locker locker { m_lock };
- bool satisfied = m_condition.waitUntil(m_lock, absoluteTime, [&] { return m_isSet; });
+ bool satisfied = m_condition.waitUntil(m_lock, absoluteTime, [&] {
+ assertIsHeld(m_lock);
+ return m_isSet;
+ });
if (satisfied)
m_isSet = false;
return satisfied;
Modified: trunk/Source/WTF/wtf/threads/BinarySemaphore.h (277921 => 277922)
--- trunk/Source/WTF/wtf/threads/BinarySemaphore.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WTF/wtf/threads/BinarySemaphore.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -25,8 +25,8 @@
#pragma once
-#include <wtf/Condition.h>
-#include <wtf/Lock.h>
+#include <wtf/CheckedCondition.h>
+#include <wtf/CheckedLock.h>
#include <wtf/Noncopyable.h>
#include <wtf/TimeWithDynamicClockType.h>
@@ -52,9 +52,9 @@
}
private:
- bool m_isSet { false };
- Lock m_lock;
- Condition m_condition;
+ bool m_isSet WTF_GUARDED_BY_LOCK(m_lock) { false };
+ CheckedLock m_lock;
+ CheckedCondition m_condition;
};
} // namespace WTF
Modified: trunk/Source/WebCore/ChangeLog (277921 => 277922)
--- trunk/Source/WebCore/ChangeLog 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/ChangeLog 2021-05-22 21:40:04 UTC (rev 277922)
@@ -1,5 +1,72 @@
2021-05-22 Chris Dumez <cdu...@apple.com>
+ Adopt CheckedLock in more places
+ https://bugs.webkit.org/show_bug.cgi?id=226138
+
+ Reviewed by Darin Adler.
+
+ Adopt CheckedLock in more places to benefit from Clang Thread Safety Analysis.
+
+ * Modules/webdatabase/Database.cpp:
+ (WebCore::Database::performClose):
+ (WebCore::Database::scheduleTransaction):
+ (WebCore::Database::inProgressTransactionCompleted):
+ (WebCore::Database::hasPendingTransaction):
+ (WebCore::Database::runTransaction):
+ * Modules/webdatabase/Database.h:
+ (WebCore::Database::WTF_GUARDED_BY_LOCK):
+ * Modules/webdatabase/DatabaseManager.cpp:
+ (WebCore::DatabaseManager::addProposedDatabase):
+ (WebCore::DatabaseManager::removeProposedDatabase):
+ (WebCore::DatabaseManager::fullPathForDatabase):
+ (WebCore::DatabaseManager::detailsForNameAndOrigin):
+ * Modules/webdatabase/DatabaseManager.h:
+ * Modules/webdatabase/DatabaseThread.cpp:
+ (WebCore::DatabaseThread::databaseThread):
+ (WebCore::DatabaseThread::recordDatabaseOpen):
+ (WebCore::DatabaseThread::recordDatabaseClosed):
+ (WebCore::DatabaseThread::hasPendingDatabaseActivity const):
+ * Modules/webdatabase/DatabaseThread.h:
+ * Modules/webdatabase/SQLCallbackWrapper.h:
+ (WebCore::SQLCallbackWrapper::clear):
+ (WebCore::SQLCallbackWrapper::unwrap):
+ * Modules/webdatabase/SQLTransaction.cpp:
+ (WebCore::SQLTransaction::enqueueStatement):
+ (WebCore::SQLTransaction::checkAndHandleClosedDatabase):
+ (WebCore::SQLTransaction::getNextStatement):
+ * Modules/webdatabase/SQLTransaction.h:
+ * Modules/webdatabase/SQLTransactionBackend.cpp:
+ (WebCore::SQLTransactionBackend::doCleanup):
+ * accessibility/isolatedtree/AXIsolatedTree.h:
+ (WebCore::AXIsolatedTree::WTF_GUARDED_BY_LOCK):
+ * platform/AbortableTaskQueue.h:
+ * platform/audio/cocoa/AudioDestinationCocoa.cpp:
+ (WebCore::AudioDestinationCocoa::render):
+ * platform/audio/cocoa/AudioDestinationCocoa.h:
+ * platform/audio/gstreamer/AudioDestinationGStreamer.h:
+ * platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
+ (WebCore::AudioSourceProviderGStreamer::provideInput):
+ (WebCore::AudioSourceProviderGStreamer::handleSample):
+ (WebCore::AudioSourceProviderGStreamer::clearAdapters):
+ * platform/audio/gstreamer/AudioSourceProviderGStreamer.h:
+ * platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:
+ (webKitWebAudioSrcRenderIteration):
+ * platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:
+ (WebCore::AVFWrapper::addToMap):
+ (WebCore::AVFWrapper::removeFromMap const):
+ (WebCore::AVFWrapper::avfWrapperForCallbackContext):
+ (WebCore::AVFWrapper::periodicTimeObserverCallback):
+ (WebCore::AVFWrapper::processNotification):
+ (WebCore::AVFWrapper::loadPlayableCompletionCallback):
+ (WebCore::AVFWrapper::loadMetadataCompletionCallback):
+ (WebCore::AVFWrapper::seekCompletedCallback):
+ (WebCore::AVFWrapper::processCue):
+ (WebCore::AVFWrapper::legibleOutputCallback):
+ (WebCore::AVFWrapper::processShouldWaitForLoadingOfResource):
+ (WebCore::AVFWrapper::resourceLoaderShouldWaitForLoadingOfRequestedResource):
+
+2021-05-22 Chris Dumez <cdu...@apple.com>
+
Use CheckedLock in SpeechRecognitionCaptureSourceImpl
https://bugs.webkit.org/show_bug.cgi?id=226131
Modified: trunk/Source/WebCore/Modules/webdatabase/Database.cpp (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/Database.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/Database.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -282,7 +282,7 @@
ASSERT(databaseThread().getThread() == &Thread::current());
{
- Locker locker { m_transactionInProgressMutex };
+ Locker locker { m_transactionInProgressLock };
// Clean up transactions that have not been scheduled yet:
// Transaction phase 1 cleanup. See comment on "What happens if a
@@ -518,7 +518,7 @@
void Database::scheduleTransaction()
{
- ASSERT(!m_transactionInProgressMutex.tryLock()); // Locked by caller.
+ ASSERT(m_transactionInProgressLock.isHeld());
if (!m_isTransactionQueueEnabled || m_transactionQueue.isEmpty()) {
m_transactionInProgress = false;
@@ -544,7 +544,7 @@
void Database::inProgressTransactionCompleted()
{
- Locker locker { m_transactionInProgressMutex };
+ Locker locker { m_transactionInProgressLock };
m_transactionInProgress = false;
scheduleTransaction();
}
@@ -551,7 +551,7 @@
bool Database::hasPendingTransaction()
{
- Locker locker { m_transactionInProgressMutex };
+ Locker locker { m_transactionInProgressLock };
return m_transactionInProgress || !m_transactionQueue.isEmpty();
}
@@ -681,7 +681,7 @@
void Database::runTransaction(RefPtr<SQLTransactionCallback>&& callback, RefPtr<SQLTransactionErrorCallback>&& errorCallback, RefPtr<VoidCallback>&& successCallback, RefPtr<SQLTransactionWrapper>&& wrapper, bool readOnly)
{
ASSERT(isMainThread());
- Locker locker { m_transactionInProgressMutex };
+ Locker locker { m_transactionInProgressLock };
if (!m_isTransactionQueueEnabled) {
if (errorCallback) {
m_document->eventLoop().queueTask(TaskSource::Networking, [errorCallback = makeRef(*errorCallback)]() {
Modified: trunk/Source/WebCore/Modules/webdatabase/Database.h (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/Database.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/Database.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -30,8 +30,8 @@
#include "ExceptionOr.h"
#include "SQLiteDatabase.h"
+#include <wtf/CheckedLock.h>
#include <wtf/Deque.h>
-#include <wtf/Lock.h>
namespace WebCore {
@@ -139,7 +139,7 @@
bool getActualVersionForTransaction(String& version);
void setEstimatedSize(unsigned long long);
- void scheduleTransaction();
+ void scheduleTransaction() WTF_REQUIRES_LOCK(m_transactionInProgressLock);
void runTransaction(RefPtr<SQLTransactionCallback>&&, RefPtr<SQLTransactionErrorCallback>&&, RefPtr<VoidCallback>&& successCallback, RefPtr<SQLTransactionWrapper>&&, bool readOnly);
@@ -169,10 +169,10 @@
Ref<DatabaseAuthorizer> m_databaseAuthorizer;
- Deque<Ref<SQLTransaction>> m_transactionQueue;
- Lock m_transactionInProgressMutex;
- bool m_transactionInProgress { false };
- bool m_isTransactionQueueEnabled { true };
+ Deque<Ref<SQLTransaction>> m_transactionQueue WTF_GUARDED_BY_LOCK(m_transactionInProgressLock);
+ CheckedLock m_transactionInProgressLock;
+ bool m_transactionInProgress WTF_GUARDED_BY_LOCK(m_transactionInProgressLock) { false };
+ bool m_isTransactionQueueEnabled WTF_GUARDED_BY_LOCK(m_transactionInProgressLock) { true };
friend class ChangeVersionWrapper;
friend class DatabaseManager;
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -181,13 +181,13 @@
void DatabaseManager::addProposedDatabase(ProposedDatabase& database)
{
- Locker locker { m_proposedDatabasesMutex };
+ Locker locker { m_proposedDatabasesLock };
m_proposedDatabases.add(&database);
}
void DatabaseManager::removeProposedDatabase(ProposedDatabase& database)
{
- Locker locker { m_proposedDatabasesMutex };
+ Locker locker { m_proposedDatabasesLock };
m_proposedDatabases.remove(&database);
}
@@ -241,7 +241,7 @@
String DatabaseManager::fullPathForDatabase(SecurityOrigin& origin, const String& name, bool createIfDoesNotExist)
{
{
- Locker locker { m_proposedDatabasesMutex };
+ Locker locker { m_proposedDatabasesLock };
for (auto* proposedDatabase : m_proposedDatabases) {
if (proposedDatabase->details().name() == name && proposedDatabase->origin().equal(&origin))
return String();
@@ -253,7 +253,7 @@
DatabaseDetails DatabaseManager::detailsForNameAndOrigin(const String& name, SecurityOrigin& origin)
{
{
- Locker locker { m_proposedDatabasesMutex };
+ Locker locker { m_proposedDatabasesLock };
for (auto* proposedDatabase : m_proposedDatabases) {
if (proposedDatabase->details().name() == name && proposedDatabase->origin().equal(&origin)) {
ASSERT(&proposedDatabase->details().thread() == &Thread::current() || isMainThread());
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.h (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseManager.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -28,8 +28,8 @@
#include "DatabaseDetails.h"
#include "ExceptionOr.h"
#include <wtf/Assertions.h>
+#include <wtf/CheckedLock.h>
#include <wtf/HashSet.h>
-#include <wtf/Lock.h>
namespace WebCore {
@@ -87,8 +87,8 @@
DatabaseManagerClient* m_client { nullptr };
bool m_databaseIsAvailable { true };
- Lock m_proposedDatabasesMutex;
- HashSet<ProposedDatabase*> m_proposedDatabases;
+ CheckedLock m_proposedDatabasesLock;
+ HashSet<ProposedDatabase*> m_proposedDatabases WTF_GUARDED_BY_LOCK(m_proposedDatabasesLock);
};
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseThread.cpp (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseThread.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseThread.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -110,7 +110,7 @@
// inconsistent or locked state.
DatabaseSet openSetCopy;
{
- Locker locker { m_openDatabaseSetMutex };
+ Locker locker { m_openDatabaseSetLock };
if (m_openDatabaseSet.size() > 0) {
// As the call to close will modify the original set, we must take a copy to iterate over.
openSetCopy.swap(m_openDatabaseSet);
@@ -134,7 +134,7 @@
void DatabaseThread::recordDatabaseOpen(Database& database)
{
- Locker locker { m_openDatabaseSetMutex };
+ Locker locker { m_openDatabaseSetLock };
ASSERT(m_thread == &Thread::current());
ASSERT(!m_openDatabaseSet.contains(&database));
@@ -143,7 +143,7 @@
void DatabaseThread::recordDatabaseClosed(Database& database)
{
- Locker locker { m_openDatabaseSetMutex };
+ Locker locker { m_openDatabaseSetLock };
ASSERT(m_thread == &Thread::current());
ASSERT(m_queue.killed() || m_openDatabaseSet.contains(&database));
@@ -172,7 +172,7 @@
bool DatabaseThread::hasPendingDatabaseActivity() const
{
- Locker locker { m_openDatabaseSetMutex };
+ Locker locker { m_openDatabaseSetLock };
for (auto& database : m_openDatabaseSet) {
if (database->hasPendingCreationEvent() || database->hasPendingTransaction())
return true;
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseThread.h (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseThread.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseThread.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -29,6 +29,7 @@
#pragma once
#include <memory>
+#include <wtf/CheckedLock.h>
#include <wtf/HashSet.h>
#include <wtf/MessageQueue.h>
#include <wtf/RefPtr.h>
@@ -75,8 +76,8 @@
// This set keeps track of the open databases that have been used on this thread.
using DatabaseSet = HashSet<RefPtr<Database>>;
- mutable Lock m_openDatabaseSetMutex;
- DatabaseSet m_openDatabaseSet;
+ mutable CheckedLock m_openDatabaseSetLock;
+ DatabaseSet m_openDatabaseSet WTF_GUARDED_BY_LOCK(m_openDatabaseSetLock);
std::unique_ptr<SQLTransactionCoordinator> m_transactionCoordinator;
DatabaseTaskSynchronizer* m_cleanupSync { nullptr };
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -104,7 +104,7 @@
void DatabaseTracker::openTrackerDatabase(TrackerCreationAction createAction)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
if (m_database.isOpen())
return;
@@ -143,7 +143,7 @@
ExceptionOr<void> DatabaseTracker::hasAdequateQuotaForOrigin(const SecurityOriginData& origin, uint64_t estimatedSize)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
auto usage = this->usage(origin);
// If the database will fit, allow its creation.
@@ -230,7 +230,7 @@
bool DatabaseTracker::hasEntryForOriginNoLock(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
openTrackerDatabase(DontCreateIfDoesNotExist);
if (!m_database.isOpen())
return false;
@@ -248,7 +248,7 @@
bool DatabaseTracker::hasEntryForDatabase(const SecurityOriginData& origin, const String& databaseIdentifier)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
openTrackerDatabase(DontCreateIfDoesNotExist);
if (!m_database.isOpen()) {
// No "tracker database". Hence, no entry for the database of interest.
@@ -313,7 +313,7 @@
String DatabaseTracker::fullPathForDatabaseNoLock(const SecurityOriginData& origin, const String& name, bool createIfNotExists)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
String originIdentifier = origin.databaseIdentifier();
String originPath = this->originPath(origin);
@@ -392,7 +392,7 @@
Vector<String> DatabaseTracker::databaseNamesNoLock(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
openTrackerDatabase(DontCreateIfDoesNotExist);
if (!m_database.isOpen())
return { };
@@ -634,7 +634,7 @@
void DatabaseTracker::deleteOriginLockFor(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
// There is not always an instance of an OriginLock associated with an origin.
// For example, if the OriginLock lock file was created by a previous run of
@@ -666,7 +666,7 @@
uint64_t DatabaseTracker::quotaNoLock(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
uint64_t quota = 0;
openTrackerDatabase(DontCreateIfDoesNotExist);
@@ -742,7 +742,7 @@
bool DatabaseTracker::addDatabase(const SecurityOriginData& origin, const String& name, const String& path)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
openTrackerDatabase(CreateIfDoesNotExist);
if (!m_database.isOpen())
return false;
@@ -946,7 +946,7 @@
bool DatabaseTracker::isDeletingDatabaseOrOriginFor(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
// Can't create a database while someone else is deleting it; there's a risk of leaving untracked database debris on the disk.
return isDeletingDatabase(origin, name) || isDeletingOrigin(origin);
}
@@ -953,7 +953,7 @@
void DatabaseTracker::recordCreatingDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
// We don't use HashMap::ensure here to avoid making an isolated copy of the origin every time.
auto* nameSet = m_beingCreated.get(origin);
@@ -967,7 +967,7 @@
void DatabaseTracker::doneCreatingDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
ASSERT(m_beingCreated.contains(origin));
@@ -984,7 +984,7 @@
bool DatabaseTracker::creatingDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
auto iterator = m_beingCreated.find(origin);
return iterator != m_beingCreated.end() && iterator->value->contains(name);
@@ -992,13 +992,13 @@
bool DatabaseTracker::canDeleteDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
return !creatingDatabase(origin, name) && !isDeletingDatabase(origin, name);
}
void DatabaseTracker::recordDeletingDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
ASSERT(canDeleteDatabase(origin, name));
// We don't use HashMap::ensure here to avoid making an isolated copy of the origin every time.
@@ -1014,7 +1014,7 @@
void DatabaseTracker::doneDeletingDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
ASSERT(m_beingDeleted.contains(origin));
auto iterator = m_beingDeleted.find(origin);
@@ -1029,7 +1029,7 @@
bool DatabaseTracker::isDeletingDatabase(const SecurityOriginData& origin, const String& name)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
auto* nameSet = m_beingDeleted.get(origin);
return nameSet && nameSet->contains(name);
}
@@ -1036,19 +1036,19 @@
bool DatabaseTracker::canDeleteOrigin(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
return !(isDeletingOrigin(origin) || m_beingCreated.get(origin));
}
bool DatabaseTracker::isDeletingOrigin(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
return m_originsBeingDeleted.contains(origin);
}
void DatabaseTracker::recordDeletingOrigin(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
ASSERT(!isDeletingOrigin(origin));
m_originsBeingDeleted.add(origin.isolatedCopy());
}
@@ -1055,7 +1055,7 @@
void DatabaseTracker::doneDeletingOrigin(const SecurityOriginData& origin)
{
- ASSERT(!m_databaseGuard.tryLock());
+ ASSERT(m_databaseGuard.isHeld());
ASSERT(isDeletingOrigin(origin));
m_originsBeingDeleted.remove(origin);
}
@@ -1167,7 +1167,8 @@
#if PLATFORM(IOS_FAMILY)
-void DatabaseTracker::removeDeletedOpenedDatabases()
+// FIXME: This uses m_database without locking m_databaseGuard.
+void DatabaseTracker::removeDeletedOpenedDatabases() WTF_IGNORES_THREAD_SAFETY_ANALYSIS
{
// This is called when another app has deleted a database. Go through all opened databases in this
// tracker and close any that's no longer being tracked in the database.
@@ -1176,10 +1177,10 @@
// Acquire the lock before calling openTrackerDatabase.
Locker lockDatabase { m_databaseGuard };
openTrackerDatabase(DontCreateIfDoesNotExist);
+
+ if (!m_database.isOpen())
+ return;
}
-
- if (!m_database.isOpen())
- return;
// Keep track of which opened databases have been deleted.
Vector<RefPtr<Database>> deletedDatabases;
@@ -1330,7 +1331,7 @@
void DatabaseTracker::scheduleForNotification()
{
- ASSERT(!notificationLock.tryLock());
+ ASSERT(notificationLock.isHeld());
if (!notificationScheduled) {
callOnMainThread([] {
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -33,6 +33,7 @@
#include "SQLiteDatabase.h"
#include "SecurityOriginData.h"
#include "SecurityOriginHash.h"
+#include <wtf/CheckedLock.h>
#include <wtf/HashCountedSet.h>
#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
@@ -119,26 +120,26 @@
private:
explicit DatabaseTracker(const String& databasePath);
- ExceptionOr<void> hasAdequateQuotaForOrigin(const SecurityOriginData&, uint64_t estimatedSize);
+ ExceptionOr<void> hasAdequateQuotaForOrigin(const SecurityOriginData&, uint64_t estimatedSize) WTF_REQUIRES_LOCK(m_databaseGuard);
- bool hasEntryForOriginNoLock(const SecurityOriginData&);
- String fullPathForDatabaseNoLock(const SecurityOriginData&, const String& name, bool createIfDoesNotExist);
- Vector<String> databaseNamesNoLock(const SecurityOriginData&);
- uint64_t quotaNoLock(const SecurityOriginData&);
+ bool hasEntryForOriginNoLock(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
+ String fullPathForDatabaseNoLock(const SecurityOriginData&, const String& name, bool createIfDoesNotExist) WTF_REQUIRES_LOCK(m_databaseGuard);
+ Vector<String> databaseNamesNoLock(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard) WTF_REQUIRES_LOCK(m_databaseGuard);
+ uint64_t quotaNoLock(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
- String trackerDatabasePath() const;
+ String trackerDatabasePath() const WTF_REQUIRES_LOCK(m_databaseGuard);
enum TrackerCreationAction {
DontCreateIfDoesNotExist,
CreateIfDoesNotExist
};
- void openTrackerDatabase(TrackerCreationAction);
+ void openTrackerDatabase(TrackerCreationAction) WTF_REQUIRES_LOCK(m_databaseGuard);
String originPath(const SecurityOriginData&) const;
- bool hasEntryForDatabase(const SecurityOriginData&, const String& databaseIdentifier);
+ bool hasEntryForDatabase(const SecurityOriginData&, const String& databaseIdentifier) WTF_REQUIRES_LOCK(m_databaseGuard);
- bool addDatabase(const SecurityOriginData&, const String& name, const String& path);
+ bool addDatabase(const SecurityOriginData&, const String& name, const String& path) WTF_REQUIRES_LOCK(m_databaseGuard);
enum class DeletionMode {
Immediate,
@@ -155,41 +156,41 @@
bool deleteOrigin(const SecurityOriginData&, DeletionMode);
bool deleteDatabaseFile(const SecurityOriginData&, const String& name, DeletionMode);
- void deleteOriginLockFor(const SecurityOriginData&);
+ void deleteOriginLockFor(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
using DatabaseSet = HashSet<Database*>;
using DatabaseNameMap = HashMap<String, DatabaseSet*>;
using DatabaseOriginMap = HashMap<SecurityOriginData, DatabaseNameMap*>;
- Lock m_openDatabaseMapGuard;
- mutable std::unique_ptr<DatabaseOriginMap> m_openDatabaseMap;
+ CheckedLock m_openDatabaseMapGuard;
+ mutable std::unique_ptr<DatabaseOriginMap> m_openDatabaseMap WTF_GUARDED_BY_LOCK(m_openDatabaseMapGuard);
// This lock protects m_database, m_originLockMap, m_databaseDirectoryPath, m_originsBeingDeleted, m_beingCreated, and m_beingDeleted.
- Lock m_databaseGuard;
- SQLiteDatabase m_database;
+ CheckedLock m_databaseGuard;
+ SQLiteDatabase m_database WTF_GUARDED_BY_LOCK(m_databaseGuard);
using OriginLockMap = HashMap<String, RefPtr<OriginLock>>;
- OriginLockMap m_originLockMap;
+ OriginLockMap m_originLockMap WTF_GUARDED_BY_LOCK(m_databaseGuard);
String m_databaseDirectoryPath;
DatabaseManagerClient* m_client { nullptr };
- HashMap<SecurityOriginData, std::unique_ptr<HashCountedSet<String>>> m_beingCreated;
- HashMap<SecurityOriginData, std::unique_ptr<HashSet<String>>> m_beingDeleted;
- HashSet<SecurityOriginData> m_originsBeingDeleted;
- bool isDeletingDatabaseOrOriginFor(const SecurityOriginData&, const String& name);
- void recordCreatingDatabase(const SecurityOriginData&, const String& name);
- void doneCreatingDatabase(const SecurityOriginData&, const String& name);
- bool creatingDatabase(const SecurityOriginData&, const String& name);
- bool canDeleteDatabase(const SecurityOriginData&, const String& name);
- void recordDeletingDatabase(const SecurityOriginData&, const String& name);
- void doneDeletingDatabase(const SecurityOriginData&, const String& name);
- bool isDeletingDatabase(const SecurityOriginData&, const String& name);
- bool canDeleteOrigin(const SecurityOriginData&);
- bool isDeletingOrigin(const SecurityOriginData&);
- void recordDeletingOrigin(const SecurityOriginData&);
- void doneDeletingOrigin(const SecurityOriginData&);
+ HashMap<SecurityOriginData, std::unique_ptr<HashCountedSet<String>>> m_beingCreated WTF_GUARDED_BY_LOCK(m_databaseGuard);
+ HashMap<SecurityOriginData, std::unique_ptr<HashSet<String>>> m_beingDeleted WTF_GUARDED_BY_LOCK(m_databaseGuard);
+ HashSet<SecurityOriginData> m_originsBeingDeleted WTF_GUARDED_BY_LOCK(m_databaseGuard);
+ bool isDeletingDatabaseOrOriginFor(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ void recordCreatingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ void doneCreatingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ bool creatingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ bool canDeleteDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ void recordDeletingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ void doneDeletingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ bool isDeletingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
+ bool canDeleteOrigin(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
+ bool isDeletingOrigin(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
+ void recordDeletingOrigin(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
+ void doneDeletingOrigin(const SecurityOriginData&) WTF_REQUIRES_LOCK(m_databaseGuard);
static void scheduleForNotification();
static void notifyDatabasesChanged();
Modified: trunk/Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -29,7 +29,7 @@
#pragma once
#include "ScriptExecutionContext.h"
-#include <wtf/Lock.h>
+#include <wtf/CheckedLock.h>
namespace WebCore {
@@ -58,7 +58,7 @@
ScriptExecutionContext* scriptExecutionContextPtr;
T* callback;
{
- Locker locker { m_mutex };
+ Locker locker { m_lock };
if (!m_callback) {
ASSERT(!m_scriptExecutionContext);
return;
@@ -83,7 +83,7 @@
RefPtr<T> unwrap()
{
- Locker locker { m_mutex };
+ Locker locker { m_lock };
ASSERT(!m_callback || m_scriptExecutionContext->isContextThread());
m_scriptExecutionContext = nullptr;
return WTFMove(m_callback);
@@ -90,12 +90,13 @@
}
// Useful for optimizations only, please test the return value of unwrap to be sure.
- bool hasCallback() const { return m_callback; }
+ // FIXME: This is not thread-safe.
+ bool hasCallback() const WTF_IGNORES_THREAD_SAFETY_ANALYSIS { return m_callback; }
private:
- Lock m_mutex;
- RefPtr<T> m_callback;
- RefPtr<ScriptExecutionContext> m_scriptExecutionContext;
+ CheckedLock m_lock;
+ RefPtr<T> m_callback WTF_GUARDED_BY_LOCK(m_lock);
+ RefPtr<ScriptExecutionContext> m_scriptExecutionContext WTF_GUARDED_BY_LOCK(m_lock);
};
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -144,7 +144,7 @@
void SQLTransaction::enqueueStatement(std::unique_ptr<SQLStatement> statement)
{
- Locker locker { m_statementMutex };
+ Locker locker { m_statementLock };
m_statementQueue.append(WTFMove(statement));
}
@@ -190,7 +190,7 @@
// If the database was stopped, don't do anything and cancel queued work
LOG(StorageAPI, "Database was stopped or interrupted - cancelling work for this transaction");
- Locker locker { m_statementMutex };
+ Locker locker { m_statementLock };
m_statementQueue.clear();
m_nextStep = nullptr;
@@ -516,7 +516,7 @@
{
m_currentStatement = nullptr;
- Locker locker { m_statementMutex };
+ Locker locker { m_statementLock };
if (!m_statementQueue.isEmpty())
m_currentStatement = m_statementQueue.takeFirst();
}
Modified: trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/SQLTransaction.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -33,6 +33,7 @@
#include "SQLTransactionBackend.h"
#include "SQLTransactionStateMachine.h"
#include "SQLValue.h"
+#include <wtf/CheckedLock.h>
#include <wtf/Deque.h>
#include <wtf/Lock.h>
@@ -138,8 +139,8 @@
bool m_readOnly { false };
bool m_hasVersionMismatch { false };
- Lock m_statementMutex;
- Deque<std::unique_ptr<SQLStatement>> m_statementQueue;
+ CheckedLock m_statementLock;
+ Deque<std::unique_ptr<SQLStatement>> m_statementQueue WTF_GUARDED_BY_LOCK(m_statementLock);
std::unique_ptr<SQLStatement> m_currentStatement;
Modified: trunk/Source/WebCore/Modules/webdatabase/SQLTransactionBackend.cpp (277921 => 277922)
--- trunk/Source/WebCore/Modules/webdatabase/SQLTransactionBackend.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/Modules/webdatabase/SQLTransactionBackend.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -357,7 +357,7 @@
m_frontend.releaseOriginLockIfNeeded();
- Locker locker { m_frontend.m_statementMutex };
+ Locker locker { m_frontend.m_statementLock };
m_frontend.m_statementQueue.clear();
if (m_frontend.m_sqliteTransaction) {
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h (277921 => 277922)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -361,7 +361,7 @@
// Both setRootNodeID and setFocusedNodeID are called during the generation
// of the IsolatedTree.
// Focused node updates in AXObjectCache use setFocusNodeID.
- void setRootNode(AXIsolatedObject*);
+ void setRootNode(AXIsolatedObject*) WTF_REQUIRES_LOCK(m_changeLogLock);
void setFocusedNodeID(AXID);
// Called on AX thread from WebAccessibilityObjectWrapper methods.
@@ -381,7 +381,7 @@
// Call on main thread
Ref<AXIsolatedObject> createSubtree(AXCoreObject&, AXID parentID, bool attachWrapper);
// Called on main thread to update both m_nodeMap and m_pendingChildrenUpdates.
- void updateChildrenIDs(AXID parentID, Vector<AXID>&& childrenIDs);
+ void updateChildrenIDs(AXID parentID, Vector<AXID>&& childrenIDs) WTF_REQUIRES_LOCK(m_changeLogLock);
AXIsolatedTreeID m_treeID;
AXObjectCache* m_axObjectCache { nullptr };
@@ -393,15 +393,15 @@
HashMap<AXID, Ref<AXIsolatedObject>> m_readerThreadNodeMap;
// Written to by main thread under lock, accessed and applied by AX thread.
- RefPtr<AXIsolatedObject> m_rootNode;
- Vector<NodeChange> m_pendingAppends; // Nodes to be added to the tree and platform-wrapped.
- Vector<AXPropertyChange> m_pendingPropertyChanges;
- Vector<AXID> m_pendingNodeRemovals; // Nodes to be removed from the tree.
- Vector<AXID> m_pendingSubtreeRemovals; // Nodes whose subtrees are to be removed from the tree.
- Vector<std::pair<AXID, Vector<AXID>>> m_pendingChildrenUpdates;
- AXID m_pendingFocusedNodeID { InvalidAXID };
+ RefPtr<AXIsolatedObject> m_rootNode WTF_GUARDED_BY_LOCK(m_changeLogLock);
+ Vector<NodeChange> m_pendingAppends WTF_GUARDED_BY_LOCK(m_changeLogLock); // Nodes to be added to the tree and platform-wrapped.
+ Vector<AXPropertyChange> m_pendingPropertyChanges WTF_GUARDED_BY_LOCK(m_changeLogLock);
+ Vector<AXID> m_pendingNodeRemovals WTF_GUARDED_BY_LOCK(m_changeLogLock); // Nodes to be removed from the tree.
+ Vector<AXID> m_pendingSubtreeRemovals WTF_GUARDED_BY_LOCK(m_changeLogLock); // Nodes whose subtrees are to be removed from the tree.
+ Vector<std::pair<AXID, Vector<AXID>>> m_pendingChildrenUpdates WTF_GUARDED_BY_LOCK(m_changeLogLock);
+ AXID m_pendingFocusedNodeID WTF_GUARDED_BY_LOCK(m_changeLogLock) { InvalidAXID };
AXID m_focusedNodeID { InvalidAXID };
- Lock m_changeLogLock;
+ CheckedLock m_changeLogLock;
};
inline AXObjectCache* AXIsolatedTree::axObjectCache() const
Modified: trunk/Source/WebCore/platform/AbortableTaskQueue.h (277921 => 277922)
--- trunk/Source/WebCore/platform/AbortableTaskQueue.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/AbortableTaskQueue.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -20,10 +20,10 @@
#pragma once
-#include <wtf/Condition.h>
+#include <wtf/CheckedCondition.h>
+#include <wtf/CheckedLock.h>
#include <wtf/Deque.h>
#include <wtf/Function.h>
-#include <wtf/Lock.h>
#include <wtf/RunLoop.h>
#include <wtf/StdLibExtras.h>
@@ -73,7 +73,7 @@
~AbortableTaskQueue()
{
ASSERT(isMainThread());
- ASSERT(!m_mutex.isHeld());
+ ASSERT(!m_lock.isHeld());
ASSERT(m_channel.isEmpty());
}
@@ -94,7 +94,7 @@
ASSERT(isMainThread());
{
- Locker locker { m_mutex };
+ Locker locker { m_lock };
m_aborting = true;
cancelAllTasks();
}
@@ -108,7 +108,7 @@
{
ASSERT(isMainThread());
- Locker locker { m_mutex };
+ Locker locker { m_lock };
ASSERT(m_aborting);
m_aborting = false;
}
@@ -123,7 +123,7 @@
{
ASSERT(!isMainThread());
- Locker locker { m_mutex };
+ Locker locker { m_lock };
if (m_aborting)
return;
@@ -143,7 +143,7 @@
// Don't deadlock the main thread with itself.
ASSERT(!isMainThread());
- Locker locker { m_mutex };
+ Locker locker { m_lock };
if (m_aborting)
return WTF::nullopt;
@@ -150,12 +150,12 @@
Optional<R> response = WTF::nullopt;
postTask([this, &response, &mainThreadTaskHandler]() {
R responseValue = mainThreadTaskHandler();
- Locker locker { m_mutex };
+ Locker locker { m_lock };
if (!m_aborting)
response = WTFMove(responseValue);
m_abortedOrResponseSet.notifyAll();
});
- m_abortedOrResponseSet.wait(m_mutex, [this, &response]() {
+ m_abortedOrResponseSet.wait(m_lock, [this, &response]() {
return m_aborting || response;
});
return response;
@@ -197,10 +197,11 @@
if (isCancelled())
return;
- Locker lock { m_taskQueue->m_mutex };
- ASSERT(this == m_taskQueue->m_channel.first().ptr());
- m_taskQueue->m_channel.removeFirst();
- lock.unlockEarly();
+ {
+ Locker lock { m_taskQueue->m_lock };
+ ASSERT(this == m_taskQueue->m_channel.first().ptr());
+ m_taskQueue->m_channel.removeFirst();
+ }
m_taskCallback();
}
@@ -213,27 +214,27 @@
{ }
};
- void postTask(WTF::Function<void()>&& callback)
+ void postTask(WTF::Function<void()>&& callback) WTF_REQUIRES_LOCK(m_lock)
{
- ASSERT(m_mutex.isHeld());
+ ASSERT(m_lock.isHeld());
Ref<Task> task = Task::create(this, WTFMove(callback));
m_channel.append(task.copyRef());
RunLoop::main().dispatch([task = WTFMove(task)]() { task->dispatch(); });
}
- void cancelAllTasks()
+ void cancelAllTasks() WTF_REQUIRES_LOCK(m_lock)
{
ASSERT(isMainThread());
- ASSERT(m_mutex.isHeld());
+ ASSERT(m_lock.isHeld());
for (Ref<Task>& task : m_channel)
task->cancel();
m_channel.clear();
}
- bool m_aborting { false };
- Lock m_mutex;
- Condition m_abortedOrResponseSet;
- WTF::Deque<Ref<Task>> m_channel;
+ bool m_aborting WTF_GUARDED_BY_LOCK(m_lock) { false };
+ CheckedLock m_lock;
+ CheckedCondition m_abortedOrResponseSet;
+ WTF::Deque<Ref<Task>> m_channel WTF_GUARDED_BY_LOCK(m_lock);
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp (277921 => 277922)
--- trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -219,10 +219,10 @@
}
// When there is a AudioWorklet, we do rendering on the AudioWorkletThread.
- auto locker = tryHoldLock(m_dispatchToRenderThreadLock);
- if (!locker)
+ if (!m_dispatchToRenderThreadLock.tryLock())
return -1;
+ Locker locker { AdoptLockTag { }, m_dispatchToRenderThreadLock };
if (!m_dispatchToRenderThread)
renderOnRenderingTheadIfPlaying(framesToRender);
else {
Modified: trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h (277921 => 277922)
--- trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -91,8 +91,8 @@
std::unique_ptr<MultiChannelResampler> m_resampler;
AudioIOPosition m_outputTimestamp;
- Lock m_dispatchToRenderThreadLock;
- Function<void(Function<void()>&&)> m_dispatchToRenderThread;
+ CheckedLock m_dispatchToRenderThreadLock;
+ Function<void(Function<void()>&&)> m_dispatchToRenderThread WTF_GUARDED_BY_LOCK(m_dispatchToRenderThreadLock);
float m_contextSampleRate;
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.h (277921 => 277922)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -58,8 +58,6 @@
GRefPtr<GstElement> m_src;
CompletionHandler<void(bool)> m_startupCompletionHandler;
CompletionHandler<void(bool)> m_stopCompletionHandler;
- Lock m_setStateLock;
- Condition m_setStateCondition;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp (277921 => 277922)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -159,9 +159,10 @@
void AudioSourceProviderGStreamer::provideInput(AudioBus* bus, size_t framesToProcess)
{
GST_TRACE("Fetching buffers from adapters");
- auto locker = tryHoldLock(m_adapterMutex);
- if (!locker)
+ if (!m_adapterLock.tryLock())
return;
+
+ Locker locker { AdoptLockTag { }, m_adapterLock };
for (auto& it : m_adapters)
copyGStreamerBuffersToAudioChannel(it.value.get(), bus, it.key - 1, framesToProcess);
}
@@ -182,7 +183,7 @@
GST_TRACE("Storing audio sample %" GST_PTR_FORMAT, sample.get());
{
- Locker locker { m_adapterMutex };
+ Locker locker { m_adapterLock };
GQuark quark = g_quark_from_static_string("channel-id");
int channelId = GPOINTER_TO_INT(g_object_get_qdata(G_OBJECT(sink), quark));
GST_DEBUG("Channel ID: %d", channelId);
@@ -384,7 +385,7 @@
void AudioSourceProviderGStreamer::clearAdapters()
{
- Locker locker { m_adapterMutex };
+ Locker locker { m_adapterLock };
for (auto& adapter : m_adapters.values())
gst_adapter_clear(adapter.get());
}
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h (277921 => 277922)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h 2021-05-22 21:40:04 UTC (rev 277922)
@@ -25,6 +25,7 @@
#include "GRefPtrGStreamer.h"
#include "MainThreadNotifier.h"
#include <gst/gst.h>
+#include <wtf/CheckedLock.h>
#include <wtf/Forward.h>
#include <wtf/Noncopyable.h>
@@ -81,11 +82,11 @@
GRefPtr<GstElement> m_audioSinkBin;
WeakPtr<AudioSourceProviderClient> m_client;
int m_deinterleaveSourcePads { 0 };
- HashMap<int, GRefPtr<GstAdapter>> m_adapters;
+ HashMap<int, GRefPtr<GstAdapter>> m_adapters WTF_GUARDED_BY_LOCK(m_adapterLock);
unsigned long m_deinterleavePadAddedHandlerId { 0 };
unsigned long m_deinterleaveNoMorePadsHandlerId { 0 };
unsigned long m_deinterleavePadRemovedHandlerId { 0 };
- Lock m_adapterMutex;
+ CheckedLock m_adapterLock;
};
}
Modified: trunk/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp (277921 => 277922)
--- trunk/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -81,8 +81,8 @@
bool hasRenderedAudibleFrame { false };
- Lock dispatchToRenderThreadLock;
- Function<void(Function<void()>&&)> dispatchToRenderThreadFunction;
+ CheckedLock dispatchToRenderThreadLock;
+ Function<void(Function<void()>&&)> dispatchToRenderThreadFunction WTF_GUARDED_BY_LOCK(dispatchToRenderThreadLock);
bool dispatchDone WTF_GUARDED_BY_LOCK(dispatchLock);
CheckedLock dispatchLock;
@@ -408,10 +408,11 @@
priv->dispatchDone = false;
}
- auto locker = tryHoldLock(priv->dispatchToRenderThreadLock);
- if (!locker)
+ if (!priv->dispatchToRenderThreadLock.tryLock())
return;
+ Locker locker { AdoptLockTag { }, priv->dispatchToRenderThreadLock };
+
if (!priv->dispatchToRenderThreadFunction)
webKitWebAudioSrcRenderAndPushFrames(GRefPtr<GstElement>(GST_ELEMENT_CAST(src)), WTFMove(*channelBufferList));
else {
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp (277921 => 277922)
--- trunk/Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -167,9 +167,9 @@
private:
inline void* callbackContext() const { return reinterpret_cast<void*>(m_objectID); }
- static Lock& mapLock();
- static HashMap<uintptr_t, AVFWrapper*>& map();
- static AVFWrapper* avfWrapperForCallbackContext(void*);
+ static CheckedLock mapLock;
+ static HashMap<uintptr_t, AVFWrapper*>& map() WTF_REQUIRES_LOCK(mapLock);
+ static AVFWrapper* avfWrapperForCallbackContext(void*) WTF_REQUIRES_LOCK(mapLock);
void addToMap();
void removeFromMap() const;
#if HAVE(AVFOUNDATION_LOADER_DELEGATE)
@@ -1416,12 +1416,7 @@
m_avPlayer = 0;
}
-Lock& AVFWrapper::mapLock()
-{
- static Lock mapLock;
- return mapLock;
-}
-
+CheckedLock AVFWrapper::mapLock;
HashMap<uintptr_t, AVFWrapper*>& AVFWrapper::map()
{
static HashMap<uintptr_t, AVFWrapper*>& map = *new HashMap<uintptr_t, AVFWrapper*>;
@@ -1430,7 +1425,7 @@
void AVFWrapper::addToMap()
{
- Locker locker { mapLock() };
+ Locker locker { mapLock };
// HashMap doesn't like a key of 0, and also make sure we aren't
// using an object ID that's already in use.
@@ -1446,13 +1441,13 @@
{
LOG(Media, "AVFWrapper::removeFromMap(%p %d)", this, m_objectID);
- Locker locker { mapLock() };
+ Locker locker { mapLock };
map().remove(m_objectID);
}
AVFWrapper* AVFWrapper::avfWrapperForCallbackContext(void* context)
{
- // Assumes caller has locked mapLock().
+ // Assumes caller has locked mapLock.
HashMap<uintptr_t, AVFWrapper*>::iterator it = map().find(reinterpret_cast<uintptr_t>(context));
if (it == map().end())
return 0;
@@ -1650,7 +1645,7 @@
void AVFWrapper::periodicTimeObserverCallback(AVCFPlayerRef, CMTime cmTime, void* context)
{
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(context);
if (!self) {
LOG(Media, "AVFWrapper::periodicTimeObserverCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1682,7 +1677,7 @@
std::unique_ptr<NotificationCallbackData> notificationData { static_cast<NotificationCallbackData*>(context) };
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(notificationData->m_context);
if (!self) {
LOG(Media, "AVFWrapper::processNotification invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1737,7 +1732,7 @@
void AVFWrapper::loadPlayableCompletionCallback(AVCFAssetRef, void* context)
{
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(context);
if (!self) {
LOG(Media, "AVFWrapper::loadPlayableCompletionCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1764,7 +1759,7 @@
void AVFWrapper::loadMetadataCompletionCallback(AVCFAssetRef, void* context)
{
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(context);
if (!self) {
LOG(Media, "AVFWrapper::loadMetadataCompletionCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1784,7 +1779,7 @@
void AVFWrapper::seekCompletedCallback(AVCFPlayerItemRef, Boolean finished, void* context)
{
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(context);
if (!self) {
LOG(Media, "AVFWrapper::seekCompletedCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1828,7 +1823,7 @@
std::unique_ptr<LegibleOutputData> legibleOutputData(reinterpret_cast<LegibleOutputData*>(context));
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(legibleOutputData->m_context);
if (!self) {
LOG(Media, "AVFWrapper::processCue invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1844,7 +1839,7 @@
void AVFWrapper::legibleOutputCallback(void* context, AVCFPlayerItemLegibleOutputRef legibleOutput, CFArrayRef attributedStrings, CFArrayRef nativeSampleBuffers, CMTime itemTime)
{
ASSERT(!isMainThread());
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(context);
if (!self) {
LOG(Media, "AVFWrapper::legibleOutputCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1883,7 +1878,7 @@
std::unique_ptr<LoadRequestData> loadRequestData(reinterpret_cast<LoadRequestData*>(context));
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(loadRequestData->m_context);
if (!self) {
LOG(Media, "AVFWrapper::processShouldWaitForLoadingOfResource invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
@@ -1938,7 +1933,7 @@
Boolean AVFWrapper::resourceLoaderShouldWaitForLoadingOfRequestedResource(AVCFAssetResourceLoaderRef resourceLoader, AVCFAssetResourceLoadingRequestRef loadingRequest, void *context)
{
ASSERT(dispatch_get_main_queue() != dispatch_get_current_queue());
- Locker locker { mapLock() };
+ Locker locker { mapLock };
AVFWrapper* self = avfWrapperForCallbackContext(context);
if (!self) {
LOG(Media, "AVFWrapper::resourceLoaderShouldWaitForLoadingOfRequestedResource invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
Modified: trunk/Source/WebKit/ChangeLog (277921 => 277922)
--- trunk/Source/WebKit/ChangeLog 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebKit/ChangeLog 2021-05-22 21:40:04 UTC (rev 277922)
@@ -1,5 +1,20 @@
2021-05-22 Chris Dumez <cdu...@apple.com>
+ Adopt CheckedLock in more places
+ https://bugs.webkit.org/show_bug.cgi?id=226138
+
+ Reviewed by Darin Adler.
+
+ Adopt CheckedLock in more places to benefit from Clang Thread Safety Analysis.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::SyncMessageState::enqueueMatchingMessages):
+ (IPC::Connection::SyncMessageState::processIncomingMessage):
+ (IPC::Connection::SyncMessageState::dispatchMessages):
+ (IPC::Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection):
+
+2021-05-22 Chris Dumez <cdu...@apple.com>
+
Replace LockHolder with Locker in local variables
https://bugs.webkit.org/show_bug.cgi?id=226133
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (277921 => 277922)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2021-05-22 20:10:40 UTC (rev 277921)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2021-05-22 21:40:04 UTC (rev 277922)
@@ -108,10 +108,10 @@
BinarySemaphore m_waitForSyncReplySemaphore;
// Protects m_didScheduleDispatchMessagesWorkSet and m_messagesToDispatchWhileWaitingForSyncReply.
- Lock m_mutex;
+ CheckedLock m_lock;
// The set of connections for which we've scheduled a call to dispatchMessageAndResetDidScheduleDispatchMessagesForConnection.
- HashSet<RefPtr<Connection>> m_didScheduleDispatchMessagesWorkSet;
+ HashSet<RefPtr<Connection>> m_didScheduleDispatchMessagesWorkSet WTF_GUARDED_BY_LOCK(m_lock);
struct ConnectionAndIncomingMessage {
Ref<Connection> connection;
@@ -123,7 +123,7 @@
}
};
Deque<ConnectionAndIncomingMessage> m_messagesBeingDispatched; // Only used on the main thread.
- Deque<ConnectionAndIncomingMessage> m_messagesToDispatchWhileWaitingForSyncReply;
+ Deque<ConnectionAndIncomingMessage> m_messagesToDispatchWhileWaitingForSyncReply WTF_GUARDED_BY_LOCK(m_lock);
};
Connection::SyncMessageState& Connection::SyncMessageState::singleton()
@@ -151,7 +151,7 @@
}
connectionAndMessages = WTFMove(rest);
};
- Locker locker { m_mutex };
+ Locker locker { m_lock };
enqueueMatchingMessagesInContainer(m_messagesBeingDispatched);
enqueueMatchingMessagesInContainer(m_messagesToDispatchWhileWaitingForSyncReply);
}
@@ -171,7 +171,7 @@
bool shouldDispatch;
{
- Locker locker { m_mutex };
+ Locker locker { m_lock };
shouldDispatch = m_didScheduleDispatchMessagesWorkSet.add(&connection).isNewEntry;
ASSERT(connection.m_incomingMessagesLock.isHeld());
if (message->shouldMaintainOrderingWithAsyncMessages()) {
@@ -198,7 +198,7 @@
ASSERT(RunLoop::isMain());
{
- Locker locker { m_mutex };
+ Locker locker { m_lock };
if (m_messagesBeingDispatched.isEmpty())
m_messagesBeingDispatched = std::exchange(m_messagesToDispatchWhileWaitingForSyncReply, { });
else {
@@ -220,7 +220,7 @@
ASSERT(RunLoop::isMain());
{
- Locker locker { m_mutex };
+ Locker locker { m_lock };
ASSERT(m_didScheduleDispatchMessagesWorkSet.contains(&connection));
m_didScheduleDispatchMessagesWorkSet.remove(&connection);
ASSERT(m_messagesBeingDispatched.isEmpty());
