diff --git a/Libraries/LibCore/EventLoopImplementationUnix.cpp b/Libraries/LibCore/EventLoopImplementationUnix.cpp index 146ce13922..1faef79f0a 100644 --- a/Libraries/LibCore/EventLoopImplementationUnix.cpp +++ b/Libraries/LibCore/EventLoopImplementationUnix.cpp @@ -271,7 +271,7 @@ struct ThreadData { s_thread_data.remove(s_thread_id); } - Sync::Mutex mutex; + Sync::RecursiveMutex mutex; // Each thread has its own timers, notifiers and a wake pipe. TimeoutSet timeouts; diff --git a/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp b/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp index 5eadd76665..d39083ae8e 100644 --- a/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp +++ b/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp @@ -12,7 +12,7 @@ namespace Audio { static PulseAudioContext* s_pulse_audio_context; -static Sync::Mutex s_pulse_audio_context_mutex; +static Sync::RecursiveMutex s_pulse_audio_context_mutex; ErrorOr> PulseAudioContext::the() { diff --git a/Libraries/LibMedia/Providers/AudioDataProvider.h b/Libraries/LibMedia/Providers/AudioDataProvider.h index 02a7d3a921..c770ccf364 100644 --- a/Libraries/LibMedia/Providers/AudioDataProvider.h +++ b/Libraries/LibMedia/Providers/AudioDataProvider.h @@ -103,7 +103,7 @@ private: void seek(AK::Duration timestamp, SeekCompletionHandler&&); - [[nodiscard]] Sync::MutexLocker take_lock() const { return Sync::MutexLocker(m_mutex); } + [[nodiscard]] Sync::MutexLocker take_lock() const { return Sync::MutexLocker(m_mutex); } void wake() const { m_wait_condition.broadcast(); } AudioDecoder const& decoder() const { return *m_decoder; } diff --git a/Libraries/LibMedia/Providers/VideoDataProvider.h b/Libraries/LibMedia/Providers/VideoDataProvider.h index 90ff833477..489a581433 100644 --- a/Libraries/LibMedia/Providers/VideoDataProvider.h +++ b/Libraries/LibMedia/Providers/VideoDataProvider.h @@ -100,7 +100,7 @@ private: TimeRanges buffered_time_ranges() const; - [[nodiscard]] Sync::MutexLocker take_lock() const { return Sync::MutexLocker(m_mutex); } + [[nodiscard]] Sync::MutexLocker take_lock() const { return Sync::MutexLocker(m_mutex); } void wake() const { m_wait_condition.broadcast(); } private: diff --git a/Libraries/LibSync/CMakeLists.txt b/Libraries/LibSync/CMakeLists.txt index 4c1bf3dc7e..32f379c6cf 100644 --- a/Libraries/LibSync/CMakeLists.txt +++ b/Libraries/LibSync/CMakeLists.txt @@ -1,5 +1,7 @@ -set(SOURCES - Mutex.cpp -) -ladybird_lib(LibSync sync EXPLICIT_SYMBOL_EXPORT) +if (WIN32) + set(SOURCES MutexWindows.cpp ConditionVariableWindows.cpp) +else() + set(SOURCES MutexPOSIX.cpp ConditionVariablePOSIX.cpp) +endif() +ladybird_lib(LibSync sync EXPLICIT_SYMBOL_EXPORT) diff --git a/Libraries/LibSync/ConditionVariable.h b/Libraries/LibSync/ConditionVariable.h index 7645baaab9..33ae7b6362 100644 --- a/Libraries/LibSync/ConditionVariable.h +++ b/Libraries/LibSync/ConditionVariable.h @@ -1,63 +1,69 @@ /* * Copyright (c) 2021, kleines Filmröllchen . + * Copyright (c) 2025, Ryszard Goc * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once +#include #include +#include +#include +#include #include -#include -#include +#include + +#if !defined(AK_OS_WINDOWS) +# include +#endif namespace Sync { -// A signaling condition variable that wraps over the pthread_cond_* APIs. -class ConditionVariable { - friend class Mutex; +// A signaling condition variable that wraps over the platform APIs. +// On posix it is a wrapper of pthread_cond_*. +// On Windows it wraps ConditionVariable +// TODO: Implement timed_wait() +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +class SYNC_API ConditionVariableBase { + AK_MAKE_NONCOPYABLE(ConditionVariableBase); + AK_MAKE_NONMOVABLE(ConditionVariableBase); public: - ConditionVariable(Mutex& to_wait_on) - : m_to_wait_on(to_wait_on) - { - auto result = pthread_cond_init(&m_condition, nullptr); - VERIFY(result == 0); - } - - ALWAYS_INLINE ~ConditionVariable() - { - auto result = pthread_cond_destroy(&m_condition); - VERIFY(result == 0); - } + ConditionVariableBase(MutexType& to_wait_on); + ~ConditionVariableBase(); // As with pthread APIs, the mutex must be locked or undefined behavior ensues. - ALWAYS_INLINE void wait() - { - auto result = pthread_cond_wait(&m_condition, &m_to_wait_on.m_mutex); - VERIFY(result == 0); - } + // Condition variables are allowed spurious wakeups. As such waiting on a condition in a loop is preferred. + void wait(); + ALWAYS_INLINE void wait_while(Function condition) { while (condition()) wait(); } // Release at least one of the threads waiting on this variable. - ALWAYS_INLINE void signal() - { - auto result = pthread_cond_signal(&m_condition); - VERIFY(result == 0); - } + void signal(); + // Release all of the threads waiting on this variable. - ALWAYS_INLINE void broadcast() - { - auto result = pthread_cond_broadcast(&m_condition); - VERIFY(result == 0); - } + void broadcast(); private: - pthread_cond_t m_condition; - Mutex& m_to_wait_on; +#ifdef AK_OS_WINDOWS + using StorageType = void*; +#else + using StorageType = pthread_cond_t; +#endif + + alignas(StorageType) unsigned char m_storage[sizeof(StorageType)]; + MutexType& m_to_wait_on; }; +template +ConditionVariableBase(MutexType&) -> ConditionVariableBase; + +using ConditionVariable = ConditionVariableBase; + } diff --git a/Libraries/LibSync/ConditionVariablePOSIX.cpp b/Libraries/LibSync/ConditionVariablePOSIX.cpp new file mode 100644 index 0000000000..bc8c0b4eaa --- /dev/null +++ b/Libraries/LibSync/ConditionVariablePOSIX.cpp @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2021, kleines Filmröllchen . + * Copyright (c) 2025, Ryszard Goc + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include + +namespace Sync { + +namespace { + +ALWAYS_INLINE pthread_cond_t* to_impl(void* ptr) +{ + return reinterpret_cast(ptr); +} + +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +ConditionVariableBase::ConditionVariableBase(MutexType& to_wait_on) + : m_to_wait_on(to_wait_on) +{ + static_assert(sizeof(m_storage) == sizeof(pthread_cond_t)); + int result = pthread_cond_init(to_impl(m_storage), nullptr); + VERIFY(result == 0); +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +ConditionVariableBase::~ConditionVariableBase() +{ + int result = pthread_cond_destroy(to_impl(m_storage)); + VERIFY(result == 0); +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +void ConditionVariableBase::wait() +{ + int result = pthread_cond_wait(to_impl(m_storage), reinterpret_cast(m_to_wait_on.m_storage)); + VERIFY(result == 0); +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +void ConditionVariableBase::signal() +{ + int result = pthread_cond_signal(to_impl(m_storage)); + VERIFY(result == 0); +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +void ConditionVariableBase::broadcast() +{ + int result = pthread_cond_broadcast(to_impl(m_storage)); + VERIFY(result == 0); +} + +template class SYNC_API ConditionVariableBase; + +} diff --git a/Libraries/LibSync/ConditionVariableWindows.cpp b/Libraries/LibSync/ConditionVariableWindows.cpp new file mode 100644 index 0000000000..ef7d4fc480 --- /dev/null +++ b/Libraries/LibSync/ConditionVariableWindows.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2025, Ryszard Goc + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include + +namespace Sync { + +namespace { + +ALWAYS_INLINE PCONDITION_VARIABLE to_impl(void* ptr) +{ + return reinterpret_cast(ptr); +} + +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +ConditionVariableBase::ConditionVariableBase(MutexType& to_wait_on) + : m_to_wait_on(to_wait_on) +{ + InitializeConditionVariable(to_impl(m_storage)); +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +ConditionVariableBase::~ConditionVariableBase() = default; + +template<> +void ConditionVariableBase::wait() +{ + BOOL result = SleepConditionVariableSRW(to_impl(m_storage), reinterpret_cast(m_to_wait_on.m_storage), INFINITE, 0); + if (!result) { + warnln("SleepConditionVariableSRW failed with: {}", Error::from_windows_error()); + VERIFY_NOT_REACHED(); + } +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +void ConditionVariableBase::signal() +{ + WakeConditionVariable(to_impl(m_storage)); +} + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +void ConditionVariableBase::broadcast() +{ + WakeAllConditionVariable(to_impl(m_storage)); +} + +template class SYNC_API ConditionVariableBase; + +} diff --git a/Libraries/LibSync/Mutex.cpp b/Libraries/LibSync/Mutex.cpp deleted file mode 100644 index bb7ab7f43b..0000000000 --- a/Libraries/LibSync/Mutex.cpp +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (c) 2018-2021, Andreas Kling - * Copyright (c) 2021, kleines Filmröllchen - * Copyright (c) 2025, Ryszard Goc - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#include - -namespace Sync { - -Mutex::Mutex() - : m_lock_count(0) -{ - pthread_mutexattr_t attr; - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&m_mutex, &attr); - pthread_mutexattr_destroy(&attr); -} - -Mutex::~Mutex() -{ - VERIFY(m_lock_count == 0); - pthread_mutex_destroy(&m_mutex); -} - -} diff --git a/Libraries/LibSync/Mutex.h b/Libraries/LibSync/Mutex.h index 5316ce9850..fdf0efd2dd 100644 --- a/Libraries/LibSync/Mutex.h +++ b/Libraries/LibSync/Mutex.h @@ -1,6 +1,7 @@ /* * Copyright (c) 2018-2021, Andreas Kling * Copyright (c) 2021, kleines Filmröllchen + * Copyright (c) 2025, Ryszard Goc * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,36 +9,81 @@ #pragma once #include +#include #include +#include #include #include -#include +#include + +#if !defined(AK_OS_WINDOWS) +# include +#endif namespace Sync { -class SYNC_API Mutex { - AK_MAKE_NONCOPYABLE(Mutex); - AK_MAKE_NONMOVABLE(Mutex); - friend class ConditionVariable; +template +class MutexBase; + +template +requires Detail::IsIntraprocess && Detail::IsNonRecursive +class ConditionVariableBase; + +template +class SYNC_API MutexBase { + AK_MAKE_NONCOPYABLE(MutexBase); + AK_MAKE_NONMOVABLE(MutexBase); + + template + requires Detail::IsIntraprocess && Detail::IsNonRecursive + friend class ConditionVariableBase; public: - Mutex(); - ~Mutex(); + using InterprocessPolicyType = InterprocessPolicy; + using RecursivePolicyType = RecursivePolicy; + + MutexBase(); + ~MutexBase(); + + bool try_lock(); void lock(); void unlock(); private: - pthread_mutex_t m_mutex; - unsigned m_lock_count { 0 }; + static consteval u64 storage_size() + { +#ifdef AK_OS_WINDOWS + if constexpr (IsSame) { + // Size of a handle + return sizeof(void*); + } + if constexpr (IsSame) { + // The size of a critical section. This is guaranteed + return 40; + } + // SRWLock is just a void* + return sizeof(void*); +#else + return sizeof(pthread_mutex_t); +#endif + } + + alignas(void*) unsigned char m_storage[storage_size()]; }; +using Mutex = MutexBase; +using RecursiveMutex = MutexBase; +using IPCMutex = MutexBase; +using IPCRecursiveMutex = MutexBase; + +template class [[nodiscard]] SYNC_API MutexLocker { AK_MAKE_NONCOPYABLE(MutexLocker); AK_MAKE_NONMOVABLE(MutexLocker); public: - ALWAYS_INLINE explicit MutexLocker(Mutex& mutex) + ALWAYS_INLINE explicit MutexLocker(MutexType& mutex) : m_mutex(mutex) { lock(); @@ -50,23 +96,10 @@ public: ALWAYS_INLINE void lock() { m_mutex.lock(); } private: - Mutex& m_mutex; + MutexType& m_mutex; }; -ALWAYS_INLINE void Mutex::lock() -{ - pthread_mutex_lock(&m_mutex); - m_lock_count++; -} - -ALWAYS_INLINE void Mutex::unlock() -{ - VERIFY(m_lock_count > 0); - // FIXME: We need to protect the lock count with the mutex itself. - // This may be bad because we're not *technically* unlocked yet, - // but we're not handling any errors from pthread_mutex_unlock anyways. - m_lock_count--; - pthread_mutex_unlock(&m_mutex); -} +template +MutexLocker(MutexType&) -> MutexLocker; } diff --git a/Libraries/LibSync/MutexPOSIX.cpp b/Libraries/LibSync/MutexPOSIX.cpp new file mode 100644 index 0000000000..a3fbb628b8 --- /dev/null +++ b/Libraries/LibSync/MutexPOSIX.cpp @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2018-2021, Andreas Kling + * Copyright (c) 2021, kleines Filmröllchen + * Copyright (c) 2025, Ryszard Goc + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace Sync { + +namespace { + +ALWAYS_INLINE pthread_mutex_t* to_impl(void* ptr) +{ + return reinterpret_cast(ptr); +} + +} + +template +MutexBase::~MutexBase() +{ + int result = pthread_mutex_destroy(to_impl(m_storage)); + if (result != 0) { + warnln("pthread_mutex_destroy failed with: {}", Error::from_errno(result)); + VERIFY_NOT_REACHED(); + } +} + +template +bool MutexBase::try_lock() +{ + int result = pthread_mutex_trylock(to_impl(m_storage)); + if (result == 0) + return true; + if (result == EBUSY) + return false; + warnln("pthread_mutex_lock failed with: {}", Error::from_errno(result)); + VERIFY_NOT_REACHED(); +} + +template +void MutexBase::lock() +{ + int result = pthread_mutex_lock(to_impl(m_storage)); + if (result != 0) { + warnln("pthread_mutex_lock failed with: {}", Error::from_errno(result)); + VERIFY_NOT_REACHED(); + } +} + +template +void MutexBase::unlock() +{ + int result = pthread_mutex_unlock(to_impl(m_storage)); + if (result != 0) { + warnln("pthread_mutex_unlock failed with: {}", Error::from_errno(result)); + VERIFY_NOT_REACHED(); + } +} + +template +MutexBase::MutexBase() +{ + static_assert(sizeof(m_storage) == sizeof(pthread_mutex_t)); + pthread_mutex_t* mutex_ptr = new (m_storage) pthread_mutex_t; + pthread_mutexattr_t attr; + pthread_mutexattr_init(&attr); + if constexpr (IsSame) { + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); + } else { + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); + } + if constexpr (IsSame) + pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED); + int result = pthread_mutex_init(mutex_ptr, &attr); + if (result != 0) { + warnln("pthread_mutex_init failed with: {}", Error::from_errno(result)); + VERIFY_NOT_REACHED(); + } + pthread_mutexattr_destroy(&attr); +} + +template class SYNC_API MutexBase; +template class SYNC_API MutexBase; +template class SYNC_API MutexBase; +template class SYNC_API MutexBase; + +} diff --git a/Libraries/LibSync/MutexProtected.h b/Libraries/LibSync/MutexProtected.h index e1c2266b3d..dffc7a4909 100644 --- a/Libraries/LibSync/MutexProtected.h +++ b/Libraries/LibSync/MutexProtected.h @@ -12,7 +12,7 @@ namespace Sync { -template +template class MutexProtected { AK_MAKE_NONCOPYABLE(MutexProtected); AK_MAKE_NONMOVABLE(MutexProtected); @@ -48,10 +48,10 @@ public: } private: - [[nodiscard]] ALWAYS_INLINE MutexLocker lock() { return MutexLocker(m_lock); } + [[nodiscard]] ALWAYS_INLINE MutexLocker lock() { return MutexLocker(m_lock); } T m_value; - Mutex m_lock {}; + MutexType m_lock {}; }; } diff --git a/Libraries/LibSync/MutexWindows.cpp b/Libraries/LibSync/MutexWindows.cpp new file mode 100644 index 0000000000..d25c7fcd19 --- /dev/null +++ b/Libraries/LibSync/MutexWindows.cpp @@ -0,0 +1,169 @@ +/* + * Copyright (c) 2025, Ryszard Goc + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include +#include + +namespace Sync { + +template<> +Mutex::MutexBase() +{ + static_assert(sizeof(m_storage) == sizeof(SRWLOCK)); + PSRWLOCK pSRW = new (m_storage) SRWLOCK; + InitializeSRWLock(pSRW); +} +template<> +Mutex::~MutexBase() = default; + +template<> +void Mutex::lock() +{ + AcquireSRWLockExclusive(reinterpret_cast(m_storage)); +} + +template<> +bool Mutex::try_lock() +{ + return TryAcquireSRWLockExclusive(reinterpret_cast(m_storage)); +} + +template<> +void Mutex::unlock() +{ + ReleaseSRWLockExclusive(reinterpret_cast(m_storage)); +} + +template<> +RecursiveMutex::MutexBase() +{ + static_assert(sizeof(m_storage) == sizeof(CRITICAL_SECTION)); + LPCRITICAL_SECTION pCS = new (m_storage) CRITICAL_SECTION; + // TODO: Optimize this for our use case + InitializeCriticalSectionAndSpinCount(pCS, 4000); +} + +template<> +RecursiveMutex::~MutexBase() +{ + DeleteCriticalSection(reinterpret_cast(m_storage)); +} + +template<> +void RecursiveMutex::lock() +{ + EnterCriticalSection(reinterpret_cast(m_storage)); +} + +template<> +bool RecursiveMutex::try_lock() +{ + return TryEnterCriticalSection(reinterpret_cast(m_storage)); +} + +template<> +void RecursiveMutex::unlock() +{ + LeaveCriticalSection(reinterpret_cast(m_storage)); +} + +namespace { + +void init_ipc_mutex(void* storage) +{ + PHANDLE pHandle = new (storage) HANDLE; + SECURITY_ATTRIBUTES sa = { .nLength = sizeof(SECURITY_ATTRIBUTES), .lpSecurityDescriptor = nullptr, .bInheritHandle = TRUE }; + // TODO: If we want to send these over IPC then some other methods have to be exposed to construct a MutexBase from + // the duplicated HANDLE. Otherwise it can use handle inheritance. We might want to only make it inheritable as an option. + HANDLE handle = CreateMutexW(&sa, FALSE, nullptr); + if (!handle) { + warnln("Failed to create mutex object with: {}", Error::from_windows_error()); + VERIFY_NOT_REACHED(); + } + *pHandle = handle; +} + +void destroy_ipc_mutex(void* storage) +{ + CloseHandle(*reinterpret_cast(storage)); +} + +void lock_ipc_mutex(void* storage) +{ + DWORD result = WaitForSingleObject(*reinterpret_cast(storage), INFINITE); + if (result != WAIT_OBJECT_0) { + warnln("Failed to acquire mutex: {}", Error::from_windows_error(result)); + VERIFY_NOT_REACHED(); + } +} + +bool try_lock_ipc_mutex(void* storage) +{ + DWORD result = WaitForSingleObject(*reinterpret_cast(storage), 0); + if (result == WAIT_OBJECT_0) { + return true; + } + if (result == WAIT_TIMEOUT) { + return false; + } + if (result == WAIT_ABANDONED) + VERIFY_NOT_REACHED(); + warnln("Failed trying to acquire mutex: {}", Error::from_windows_error(result)); + VERIFY_NOT_REACHED(); +} + +void unlock_ipc_mutex(void* storage) +{ + BOOL result = ReleaseMutex(*reinterpret_cast(storage)); + if (!result) { + warnln("Failed to release mutex: {}", Error::from_windows_error()); + VERIFY_NOT_REACHED(); + } +} + +} + +template<> +IPCMutex::MutexBase() +{ + static_assert(sizeof(m_storage) == sizeof(HANDLE)); + init_ipc_mutex(m_storage); +} +template<> +IPCMutex::~MutexBase() { destroy_ipc_mutex(m_storage); } +template<> +void IPCMutex::lock() { lock_ipc_mutex(m_storage); } +template<> +bool IPCMutex::try_lock() { return try_lock_ipc_mutex(m_storage); } +template<> +void IPCMutex::unlock() { unlock_ipc_mutex(m_storage); } + +template<> +IPCRecursiveMutex::MutexBase() +{ + static_assert(sizeof(m_storage) == sizeof(HANDLE)); + init_ipc_mutex(m_storage); +} +template<> +IPCRecursiveMutex::~MutexBase() { destroy_ipc_mutex(m_storage); } +template<> +void IPCRecursiveMutex::lock() { lock_ipc_mutex(m_storage); } +template<> +bool IPCRecursiveMutex::try_lock() { return try_lock_ipc_mutex(m_storage); } +template<> +void IPCRecursiveMutex::unlock() { unlock_ipc_mutex(m_storage); } + +template class SYNC_API MutexBase; +template class SYNC_API MutexBase; +template class SYNC_API MutexBase; +template class SYNC_API MutexBase; + +} diff --git a/Libraries/LibSync/Policy.h b/Libraries/LibSync/Policy.h new file mode 100644 index 0000000000..4f513aa887 --- /dev/null +++ b/Libraries/LibSync/Policy.h @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2025, Ryszard Goc + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace Sync { + +struct PolicyNonRecursive { }; +struct PolicyRecursive { }; +struct PolicyIntraprocess { }; +struct PolicyInterprocess { }; + +namespace Detail { + +template +concept IsIntraprocess = requires { + requires IsSame; +}; + +template +concept IsNonRecursive = requires { + requires IsSame; +}; + +} +}