From 46174348550de04e89302356f2d0481089786baf Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 6 Dec 2021 13:10:52 -0800 Subject: [PATCH] Add unconditional waits on fml::Semaphore. The class was also undocumented. So the docstrings have been filled in. This will become necessary for waiting on command buffer submission on the CPU. --- fml/synchronization/semaphore.cc | 27 ++++++++++++ fml/synchronization/semaphore.h | 51 +++++++++++++++++++++++ fml/synchronization/semaphore_unittest.cc | 20 ++++++++- 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/fml/synchronization/semaphore.cc b/fml/synchronization/semaphore.cc index 548731d6700ea..7156e2ec88171 100644 --- a/fml/synchronization/semaphore.cc +++ b/fml/synchronization/semaphore.cc @@ -29,6 +29,13 @@ class PlatformSemaphore { bool IsValid() const { return _sem != nullptr; } + bool Wait() { + if (_sem == nullptr) { + return false; + } + return dispatch_semaphore_wait(_sem, DISPATCH_TIME_FOREVER) == 0; + } + bool TryWait() { if (_sem == nullptr) { return false; @@ -71,6 +78,14 @@ class PlatformSemaphore { bool IsValid() const { return _sem != nullptr; } + bool Wait() { + if (_sem == nullptr) { + return false; + } + + return WaitForSingleObject(_sem, INFINITE) == WAIT_OBJECT_0; + } + bool TryWait() { if (_sem == nullptr) { return false; @@ -116,6 +131,14 @@ class PlatformSemaphore { bool IsValid() const { return valid_; } + bool Wait() { + if (!valid_) { + return false; + } + + return FML_HANDLE_EINTR(::sem_wait(&sem_)) == 0; + } + bool TryWait() { if (!valid_) { return false; @@ -155,6 +178,10 @@ bool Semaphore::IsValid() const { return _impl->IsValid(); } +bool Semaphore::Wait() { + return _impl->Wait(); +} + bool Semaphore::TryWait() { return _impl->TryWait(); } diff --git a/fml/synchronization/semaphore.h b/fml/synchronization/semaphore.h index 0b0dda5cfed4b..b88be468ee27c 100644 --- a/fml/synchronization/semaphore.h +++ b/fml/synchronization/semaphore.h @@ -14,16 +14,67 @@ namespace fml { class PlatformSemaphore; +//------------------------------------------------------------------------------ +/// @brief A traditional counting semaphore. `Wait`s decrement the counter +/// and `Signal` increments it. +/// +/// This is a cross-platform replacement for std::counting_semaphore +/// which is only available since C++20. Once Flutter migrates past +/// that point, this class should become obsolete and must be +/// replaced. +/// class Semaphore { public: + //---------------------------------------------------------------------------- + /// @brief Initializes the counting semaphore to a specified start count. + /// + /// @warning Callers must check if the handle could be successfully created + /// by calling the `IsValid` method. `Wait`s on an invalid + /// semaphore will always fail and signals will fail silently. + /// + /// @param[in] count The starting count of the counting semaphore. + /// explicit Semaphore(uint32_t count); + //---------------------------------------------------------------------------- + /// @brief Destroy the counting semaphore. + /// ~Semaphore(); + //---------------------------------------------------------------------------- + /// @brief Check if the underlying semaphore handle could be created. + /// Failure modes are platform specific and may occur due to issue + /// like handle exhaustion. All `Wait`s on invalid semaphore + /// handles will fail and `Signal` calls will be ignored. + /// + /// @return True if valid, False otherwise. + /// bool IsValid() const; + //---------------------------------------------------------------------------- + /// @brief Decrements the count and waits indefinitely if the value is + /// less than zero for a `Signal`. + /// + /// @return If the `Wait` call was successful. See `IsValid` for failure. + /// + [[nodiscard]] bool Wait(); + + //---------------------------------------------------------------------------- + /// @brief Decrement the counts if it is greater than zero. Returns false + /// if the counter is already at zero. + /// + /// @warning False is also returned if the semaphore handle is invalid. + /// Which makes doing the validity check before this call doubly + /// important. + /// + /// @return If the count could be decrement. + /// [[nodiscard]] bool TryWait(); + //---------------------------------------------------------------------------- + /// @brief Increment the count by one. Any pending `Wait`s will be + /// resolved at this point. + /// void Signal(); private: diff --git a/fml/synchronization/semaphore_unittest.cc b/fml/synchronization/semaphore_unittest.cc index dbf9a4cfd0c4a..384498d225340 100644 --- a/fml/synchronization/semaphore_unittest.cc +++ b/fml/synchronization/semaphore_unittest.cc @@ -2,10 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/fml/synchronization/semaphore.h" - #include +#include "flutter/fml/synchronization/semaphore.h" +#include "flutter/fml/thread.h" +#include "flutter/fml/time/time_point.h" #include "gtest/gtest.h" TEST(SemaphoreTest, SimpleValidity) { @@ -26,3 +27,18 @@ TEST(SemaphoreTest, WaitOnZeroSignalThenWait) { ASSERT_TRUE(sem.TryWait()); ASSERT_FALSE(sem.TryWait()); } + +TEST(SemaphoreTest, IndefiniteWait) { + auto start = fml::TimePoint::Now(); + constexpr double wait_in_seconds = 0.25; + fml::Semaphore sem(0); + ASSERT_TRUE(sem.IsValid()); + fml::Thread signaller("signaller_thread"); + signaller.GetTaskRunner()->PostTaskForTime( + [&sem]() { sem.Signal(); }, + start + fml::TimeDelta::FromSecondsF(wait_in_seconds)); + ASSERT_TRUE(sem.Wait()); + auto delta = fml::TimePoint::Now() - start; + ASSERT_GE(delta.ToSecondsF(), wait_in_seconds); + signaller.Join(); +}