Add unconditional waits on fml::Semaphore. (flutter#30165)

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();
 }

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:

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 <thread>
 
+#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();
+}