From c2acef195870782350d8e8daf1714dedca0efa48 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Sep 2022 16:01:38 -0700 Subject: [PATCH] Revert "fix promise" This reverts commit ded85e7d198da5d28ab75aa8126df248b9a68158. --- src/core/lib/event_engine/promise.h | 13 ++++++++----- test/core/event_engine/common_closures_test.cc | 8 ++++---- .../event_engine/posix/event_poller_posix_test.cc | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/core/lib/event_engine/promise.h b/src/core/lib/event_engine/promise.h index c92c95d0f1d..897d2cf7296 100644 --- a/src/core/lib/event_engine/promise.h +++ b/src/core/lib/event_engine/promise.h @@ -25,8 +25,8 @@ namespace experimental { /// A minimal promise implementation. /// /// This is light-duty, syntactical sugar around cv wait & signal, which is -/// useful in some cases. -/// TODO(ctiller): Find a new name for this type. +/// useful in some cases. A more robust implementation is being worked on +/// separately. template class Promise { public: @@ -36,10 +36,13 @@ class Promise { explicit Promise(T&& val) : val_(val) {} // The getter will wait until the setter has been called, and will return the // value passed during Set. - T& Get() { + T& Get() { return WaitWithTimeout(absl::Hours(1)); } + // The getter will wait with timeout until the setter has been called, and + // will return the value passed during Set. + T& WaitWithTimeout(absl::Duration d) { grpc_core::MutexLock lock(&mu_); - while (!set_) { - cv_.Wait(&mu_); + if (!set_) { + cv_.WaitWithTimeout(&mu_, d); } return val_; } diff --git a/test/core/event_engine/common_closures_test.cc b/test/core/event_engine/common_closures_test.cc index e9a9ada29c1..34f10b9befd 100644 --- a/test/core/event_engine/common_closures_test.cc +++ b/test/core/event_engine/common_closures_test.cc @@ -31,7 +31,7 @@ TEST_F(AnyInvocableClosureTest, CallsItsFunction) { Promise promise; AnyInvocableClosure closure([&promise] { promise.Set(true); }); closure.Run(); - ASSERT_TRUE(promise.Get()); + ASSERT_TRUE(promise.WaitWithTimeout(absl::Seconds(3))); } class SelfDeletingClosureTest : public testing::Test {}; @@ -41,7 +41,7 @@ TEST_F(SelfDeletingClosureTest, CallsItsFunction) { auto* closure = SelfDeletingClosure::Create([&promise] { promise.Set(true); }); closure->Run(); - ASSERT_TRUE(promise.Get()); + ASSERT_TRUE(promise.WaitWithTimeout(absl::Seconds(3))); // ASAN should catch if this closure is not deleted } @@ -52,8 +52,8 @@ TEST_F(SelfDeletingClosureTest, CallsItsFunctionAndIsDestroyed) { SelfDeletingClosure::Create([&fn_called] { fn_called.Set(true); }, [&destroyed] { destroyed.Set(true); }); closure->Run(); - ASSERT_TRUE(fn_called.Get()); - ASSERT_TRUE(destroyed.Get()); + ASSERT_TRUE(fn_called.WaitWithTimeout(absl::Seconds(3))); + ASSERT_TRUE(destroyed.WaitWithTimeout(absl::Seconds(3))); } int main(int argc, char** argv) { diff --git a/test/core/event_engine/posix/event_poller_posix_test.cc b/test/core/event_engine/posix/event_poller_posix_test.cc index 77a6338bbd0..300a5b84c1b 100644 --- a/test/core/event_engine/posix/event_poller_posix_test.cc +++ b/test/core/event_engine/posix/event_poller_posix_test.cc @@ -680,7 +680,7 @@ class Worker : public grpc_core::DualRefCounted { } void Wait() { - EXPECT_TRUE(promise.Get()); + EXPECT_TRUE(promise.WaitWithTimeout(absl::Seconds(60))); WeakUnref(); }