Add different behavior to AwakableList for "persistent" vs "one-shot" awakables. * See the comment above AwakableList::Add()'s declaration for details about the distinction between persistent and one-shot. * Rename various |force| parameters to (Add()/AddAwakable()/etc.) to |persistent|. * A lot more work has to be done in WaitSetDispatcher, but this change is already too big, so I did the minimum amount to keep it sane. * Also, more tests should be added. R=vardhan@google.com BUG=#350 Review URL: https://codereview.chromium.org/2088833003 .
diff --git a/mojo/edk/system/async_waiter.cc b/mojo/edk/system/async_waiter.cc index 15122a3..130fcc0 100644 --- a/mojo/edk/system/async_waiter.cc +++ b/mojo/edk/system/async_waiter.cc
@@ -11,12 +11,11 @@ AsyncWaiter::~AsyncWaiter() {} -bool AsyncWaiter::Awake(uint64_t /*context*/, +void AsyncWaiter::Awake(uint64_t /*context*/, AwakeReason reason, const HandleSignalsState& /*signals_state*/) { callback_(MojoResultForAwakeReason(reason)); delete this; - return false; } } // namespace system
diff --git a/mojo/edk/system/async_waiter.h b/mojo/edk/system/async_waiter.h index 5e405bf..fd8e2bc 100644 --- a/mojo/edk/system/async_waiter.h +++ b/mojo/edk/system/async_waiter.h
@@ -14,7 +14,9 @@ namespace mojo { namespace system { -// An |Awakable| implementation that just calls a given callback object. +// An |Awakable| implementation that just calls a given callback object. It +// should be used in a non-persistent way (i.e., |Awake()| should be called at +// most once by each source, and only for "leading edges"). class AsyncWaiter final : public Awakable { public: using AwakeCallback = std::function<void(MojoResult)>; @@ -25,7 +27,7 @@ private: // |Awakable| implementation: - bool Awake(uint64_t context, + void Awake(uint64_t context, AwakeReason reason, const HandleSignalsState& signals_state) override;
diff --git a/mojo/edk/system/awakable.cc b/mojo/edk/system/awakable.cc index f3d2336..39539dd 100644 --- a/mojo/edk/system/awakable.cc +++ b/mojo/edk/system/awakable.cc
@@ -18,6 +18,8 @@ return MOJO_RESULT_FAILED_PRECONDITION; case AwakeReason::CANCELLED: return MOJO_RESULT_CANCELLED; + case AwakeReason::CHANGED: + break; } NOTREACHED(); return MOJO_RESULT_INTERNAL;
diff --git a/mojo/edk/system/awakable.h b/mojo/edk/system/awakable.h index cbd0240..b4524f2 100644 --- a/mojo/edk/system/awakable.h +++ b/mojo/edk/system/awakable.h
@@ -17,21 +17,24 @@ // implementation that blocks while waiting to be awoken. class Awakable { public: - enum class AwakeReason { SATISFIED, UNSATISFIABLE, CANCELLED }; + // See |AwakableList| (in particular its |Add()| method). + enum class AwakeReason { SATISFIED, UNSATISFIABLE, CANCELLED, CHANGED }; // Helper function that translates: // - |AwakeReason::SATISFIED| -> |MOJO_RESULT_OK|, // - |AwakeReason::UNSATISFIABLE| -> |MOJO_RESULT_FAILED_PRECONDITION|, and // - |AwakeReason::CANCELLED| -> |MOJO_RESULT_CANCELLED|. + // - |AwakeReason::CHANGED| -> |MOJO_RESULT_INTERNAL| (this function never + // be called with this reason). static MojoResult MojoResultForAwakeReason(AwakeReason reason); // |Awake()| must satisfy the following contract: // - It must be thread-safe. // - Since it is called with a mutex held, it must not call anything that // takes "non-terminal" locks, i.e., those which are always safe to take. - // - It should return false if it must not be called again for the same - // reason (e.g., for the same call to |AwakableList::Add()|). - virtual bool Awake(uint64_t context, + // If |reason| is |AwakeReason::CANCELLED|, |Awake()| will not be called + // again (by the same source). + virtual void Awake(uint64_t context, AwakeReason reason, const HandleSignalsState& signals_state) = 0;
diff --git a/mojo/edk/system/awakable_list.cc b/mojo/edk/system/awakable_list.cc index fb432b9..3c3799a 100644 --- a/mojo/edk/system/awakable_list.cc +++ b/mojo/edk/system/awakable_list.cc
@@ -24,18 +24,35 @@ // Instead of deleting elements in-place, swap them with the last element and // erase the elements from the end. auto last = awakables_.end(); - for (AwakeInfoList::iterator it = awakables_.begin(); it != last;) { - bool keep = true; - if (new_state.satisfies(it->signals) && !old_state.satisfies(it->signals)) { - keep = it->awakable->Awake(it->context, Awakable::AwakeReason::SATISFIED, - new_state); - } else if (!new_state.can_satisfy(it->signals) && - old_state.can_satisfy(it->signals)) { - keep = it->awakable->Awake( - it->context, Awakable::AwakeReason::UNSATISFIABLE, new_state); + for (auto it = awakables_.begin(); it != last;) { + bool awoken = false; + if (it->persistent) { + // Persistent awakables are called for all changes on watched signals. + if ((new_state.satisfied_signals & it->signals) != + (old_state.satisfied_signals & it->signals) || + (new_state.satisfiable_signals & it->signals) != + (old_state.satisfiable_signals & it->signals)) { + awoken = true; + it->awakable->Awake(it->context, Awakable::AwakeReason::CHANGED, + new_state); + } + } else { + // One-shot awakables are only called on "leading edge" changes. + if (new_state.satisfies(it->signals) && + !old_state.satisfies(it->signals)) { + awoken = true; + it->awakable->Awake(it->context, Awakable::AwakeReason::SATISFIED, + new_state); + } else if (!new_state.can_satisfy(it->signals) && + old_state.can_satisfy(it->signals)) { + awoken = true; + it->awakable->Awake(it->context, Awakable::AwakeReason::UNSATISFIABLE, + new_state); + } } - if (!keep) { + // Remove if the awakable was awoken and one-shot. + if (awoken && !it->persistent) { --last; std::swap(*it, *last); } else { @@ -45,9 +62,8 @@ awakables_.erase(last, awakables_.end()); } -void AwakableList::CancelAll() { - for (AwakeInfoList::iterator it = awakables_.begin(); it != awakables_.end(); - ++it) { +void AwakableList::CancelAndRemoveAll() { + for (auto it = awakables_.begin(); it != awakables_.end(); ++it) { it->awakable->Awake(it->context, Awakable::AwakeReason::CANCELLED, HandleSignalsState()); } @@ -56,8 +72,9 @@ void AwakableList::Add(Awakable* awakable, uint64_t context, + bool persistent, MojoHandleSignals signals) { - awakables_.push_back(AwakeInfo(awakable, signals, context)); + awakables_.push_back(AwakeInfo(awakable, context, persistent, signals)); } void AwakableList::Remove(bool match_context,
diff --git a/mojo/edk/system/awakable_list.h b/mojo/edk/system/awakable_list.h index d1e947d..83b6093 100644 --- a/mojo/edk/system/awakable_list.h +++ b/mojo/edk/system/awakable_list.h
@@ -32,10 +32,27 @@ void OnStateChange(const HandleSignalsState& old_state, const HandleSignalsState& new_state); - void CancelAll(); + // This will awake all awakables with |Awakable::AwakeReason::CANCELLED|, and + // remove all awakes. + void CancelAndRemoveAll(); // Adds an awakable, identified by its pointer and its context. - void Add(Awakable* awakable, uint64_t context, MojoHandleSignals signals); + // + // An awakable may either be persistent or one-shot (non-persistent). + // - A one-shot's |Awake()| will be called at most once per |Add()|, and + // will only be called if a watched signal goes from unsatisfied to + // satisfied (|Awake()| will be called with reason + // |Awakable::AwakeReason::SATISFIED|), all watched signals become + // never-satisfiable (|Awakable::AwakeReason::UNSATISFIABLE|), or + // |CancelAndRemoveAll()| is called (|Awakable::AwakeReason::CANCELLED|). + // - A persistent awakable's |Awake()| will be called for all state changes + // on watched signals (with reason |Awakable::AwakeReason::CHANGED|) until + // |CancelAndRemoveAll()| is called (at which point its |Awake()| will be + // called a final time with reason |Awakable::AwakeReason::CANCELLED|). + void Add(Awakable* awakable, + uint64_t context, + bool persistent, + MojoHandleSignals signals); // Removes all awakables matching the given pointer and, if |match_context| is // true, the given context. @@ -43,12 +60,19 @@ private: struct AwakeInfo { - AwakeInfo(Awakable* awakable, MojoHandleSignals signals, uint64_t context) - : awakable(awakable), signals(signals), context(context) {} + AwakeInfo(Awakable* awakable, + uint64_t context, + bool persistent, + MojoHandleSignals signals) + : awakable(awakable), + context(context), + persistent(persistent), + signals(signals) {} Awakable* awakable; - MojoHandleSignals signals; uint64_t context; + bool persistent; + MojoHandleSignals signals; }; using AwakeInfoList = std::vector<AwakeInfo>;
diff --git a/mojo/edk/system/awakable_list_unittest.cc b/mojo/edk/system/awakable_list_unittest.cc index 2f8c619..497478c 100644 --- a/mojo/edk/system/awakable_list_unittest.cc +++ b/mojo/edk/system/awakable_list_unittest.cc
@@ -30,9 +30,9 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 1, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 1, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); - awakable_list.CancelAll(); + awakable_list.CancelAndRemoveAll(); // Double-remove okay: awakable_list.Remove(false, thread.waiter(), 0); } // Join |thread|. @@ -43,8 +43,8 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 2, MOJO_HANDLE_SIGNAL_WRITABLE); - awakable_list.CancelAll(); + awakable_list.Add(thread.waiter(), 2, false, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.CancelAndRemoveAll(); thread.Start(); } // Join |thread|. EXPECT_EQ(MOJO_RESULT_CANCELLED, result); @@ -54,10 +54,10 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 3, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 3, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); ThreadSleep(2 * test::EpsilonTimeout()); - awakable_list.CancelAll(); + awakable_list.CancelAndRemoveAll(); } // Join |thread|. EXPECT_EQ(MOJO_RESULT_CANCELLED, result); EXPECT_EQ(3u, context); @@ -71,7 +71,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 1, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 1, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); awakable_list.OnStateChange( HandleSignalsState( @@ -89,7 +89,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 2, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread.waiter(), 2, false, MOJO_HANDLE_SIGNAL_WRITABLE); awakable_list.OnStateChange( HandleSignalsState( MOJO_HANDLE_SIGNAL_NONE, @@ -109,7 +109,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 3, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 3, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); ThreadSleep(2 * test::EpsilonTimeout()); awakable_list.OnStateChange( @@ -133,7 +133,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 1, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 1, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); awakable_list.OnStateChange( HandleSignalsState( @@ -150,7 +150,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 2, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread.waiter(), 2, false, MOJO_HANDLE_SIGNAL_WRITABLE); awakable_list.OnStateChange( HandleSignalsState( MOJO_HANDLE_SIGNAL_NONE, @@ -167,7 +167,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 3, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 3, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); ThreadSleep(2 * test::EpsilonTimeout()); awakable_list.OnStateChange( @@ -198,13 +198,13 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread1(&result1, &context1); - awakable_list.Add(thread1.waiter(), 1, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread1.waiter(), 1, false, MOJO_HANDLE_SIGNAL_READABLE); thread1.Start(); test::SimpleWaiterThread thread2(&result2, &context2); - awakable_list.Add(thread2.waiter(), 2, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread2.waiter(), 2, false, MOJO_HANDLE_SIGNAL_WRITABLE); thread2.Start(); ThreadSleep(2 * test::EpsilonTimeout()); - awakable_list.CancelAll(); + awakable_list.CancelAndRemoveAll(); } // Join threads. EXPECT_EQ(MOJO_RESULT_CANCELLED, result1); EXPECT_EQ(1u, context1); @@ -215,10 +215,10 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread1(&result1, &context1); - awakable_list.Add(thread1.waiter(), 3, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread1.waiter(), 3, false, MOJO_HANDLE_SIGNAL_READABLE); thread1.Start(); test::SimpleWaiterThread thread2(&result2, &context2); - awakable_list.Add(thread2.waiter(), 4, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread2.waiter(), 4, false, MOJO_HANDLE_SIGNAL_WRITABLE); thread2.Start(); ThreadSleep(2 * test::EpsilonTimeout()); awakable_list.OnStateChange( @@ -229,7 +229,7 @@ MOJO_HANDLE_SIGNAL_READABLE, MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE)); awakable_list.Remove(false, thread1.waiter(), 0); - awakable_list.CancelAll(); + awakable_list.CancelAndRemoveAll(); } // Join threads. EXPECT_EQ(MOJO_RESULT_OK, result1); EXPECT_EQ(3u, context1); @@ -240,10 +240,10 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread1(&result1, &context1); - awakable_list.Add(thread1.waiter(), 5, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread1.waiter(), 5, false, MOJO_HANDLE_SIGNAL_READABLE); thread1.Start(); test::SimpleWaiterThread thread2(&result2, &context2); - awakable_list.Add(thread2.waiter(), 6, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread2.waiter(), 6, false, MOJO_HANDLE_SIGNAL_WRITABLE); thread2.Start(); ThreadSleep(2 * test::EpsilonTimeout()); awakable_list.OnStateChange( @@ -253,7 +253,7 @@ HandleSignalsState(MOJO_HANDLE_SIGNAL_NONE, MOJO_HANDLE_SIGNAL_READABLE)); awakable_list.Remove(false, thread2.waiter(), 0); - awakable_list.CancelAll(); + awakable_list.CancelAndRemoveAll(); } // Join threads. EXPECT_EQ(MOJO_RESULT_CANCELLED, result1); EXPECT_EQ(5u, context1); @@ -264,7 +264,7 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread1(&result1, &context1); - awakable_list.Add(thread1.waiter(), 7, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread1.waiter(), 7, false, MOJO_HANDLE_SIGNAL_READABLE); thread1.Start(); ThreadSleep(1 * test::EpsilonTimeout()); @@ -279,7 +279,7 @@ MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE)); test::SimpleWaiterThread thread2(&result2, &context2); - awakable_list.Add(thread2.waiter(), 8, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread2.waiter(), 8, false, MOJO_HANDLE_SIGNAL_WRITABLE); thread2.Start(); ThreadSleep(1 * test::EpsilonTimeout()); @@ -297,11 +297,11 @@ ThreadSleep(1 * test::EpsilonTimeout()); test::SimpleWaiterThread thread3(&result3, &context3); - awakable_list.Add(thread3.waiter(), 9, MOJO_HANDLE_SIGNAL_WRITABLE); + awakable_list.Add(thread3.waiter(), 9, false, MOJO_HANDLE_SIGNAL_WRITABLE); thread3.Start(); test::SimpleWaiterThread thread4(&result4, &context4); - awakable_list.Add(thread4.waiter(), 10, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread4.waiter(), 10, false, MOJO_HANDLE_SIGNAL_READABLE); thread4.Start(); ThreadSleep(1 * test::EpsilonTimeout()); @@ -317,7 +317,7 @@ awakable_list.Remove(false, thread3.waiter(), 0); // Cancel #4. - awakable_list.CancelAll(); + awakable_list.CancelAndRemoveAll(); } // Join threads. EXPECT_EQ(MOJO_RESULT_OK, result1); EXPECT_EQ(7u, context1); @@ -336,8 +336,8 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 1, MOJO_HANDLE_SIGNAL_READABLE); - awakable_list.Add(thread.waiter(), 2, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 1, false, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 2, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); awakable_list.Remove(true, thread.waiter(), 2); awakable_list.OnStateChange( @@ -358,8 +358,8 @@ { AwakableList awakable_list; test::SimpleWaiterThread thread(&result, &context); - awakable_list.Add(thread.waiter(), 1, MOJO_HANDLE_SIGNAL_READABLE); - awakable_list.Add(thread.waiter(), 2, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 1, false, MOJO_HANDLE_SIGNAL_READABLE); + awakable_list.Add(thread.waiter(), 2, false, MOJO_HANDLE_SIGNAL_READABLE); thread.Start(); awakable_list.Remove(true, thread.waiter(), 1); awakable_list.OnStateChange( @@ -375,86 +375,69 @@ EXPECT_EQ(2u, context); } -class KeepAwakable : public Awakable { +class TestAwakable : public Awakable { public: - KeepAwakable() : awake_count(0) {} + TestAwakable() {} - bool Awake(uint64_t context, - AwakeReason reason, - const HandleSignalsState& signals_state) override { - awake_count++; - return true; - } - - int awake_count; - - MOJO_DISALLOW_COPY_AND_ASSIGN(KeepAwakable); -}; - -class RemoveAwakable : public Awakable { - public: - RemoveAwakable() : awake_count(0) {} - - bool Awake(uint64_t /*context*/, + void Awake(uint64_t /*context*/, AwakeReason /*reason*/, const HandleSignalsState& /*signals_state*/) override { awake_count++; - return false; } - int awake_count; + unsigned awake_count = 0; - MOJO_DISALLOW_COPY_AND_ASSIGN(RemoveAwakable); + MOJO_DISALLOW_COPY_AND_ASSIGN(TestAwakable); }; -TEST(AwakableListTest, KeepAwakablesReturningTrue) { - KeepAwakable keep0; - KeepAwakable keep1; - RemoveAwakable remove0; - RemoveAwakable remove1; - RemoveAwakable remove2; +TEST(AwakableListTest, PersistentVsNonPersistent) { + TestAwakable persistent0; + TestAwakable persistent1; + TestAwakable oneshot0; + TestAwakable oneshot1; + TestAwakable oneshot2; AwakableList remove_all; - remove_all.Add(&remove0, 0, MOJO_HANDLE_SIGNAL_WRITABLE); - remove_all.Add(&remove1, 0, MOJO_HANDLE_SIGNAL_WRITABLE); + remove_all.Add(&oneshot0, 0, false, MOJO_HANDLE_SIGNAL_WRITABLE); + remove_all.Add(&oneshot1, 0, false, MOJO_HANDLE_SIGNAL_WRITABLE); remove_all.OnStateChange( HandleSignalsState(MOJO_HANDLE_SIGNAL_NONE, MOJO_HANDLE_SIGNAL_WRITABLE), HandleSignalsState(MOJO_HANDLE_SIGNAL_WRITABLE, MOJO_HANDLE_SIGNAL_WRITABLE)); - EXPECT_EQ(remove0.awake_count, 1); - EXPECT_EQ(remove1.awake_count, 1); + EXPECT_EQ(oneshot0.awake_count, 1u); + EXPECT_EQ(oneshot1.awake_count, 1u); remove_all.OnStateChange( HandleSignalsState(MOJO_HANDLE_SIGNAL_NONE, MOJO_HANDLE_SIGNAL_WRITABLE), HandleSignalsState(MOJO_HANDLE_SIGNAL_WRITABLE, MOJO_HANDLE_SIGNAL_WRITABLE)); - EXPECT_EQ(remove0.awake_count, 1); - EXPECT_EQ(remove1.awake_count, 1); + EXPECT_EQ(oneshot0.awake_count, 1u); + EXPECT_EQ(oneshot1.awake_count, 1u); AwakableList remove_first; - remove_first.Add(&remove2, 0, MOJO_HANDLE_SIGNAL_WRITABLE); - remove_first.Add(&keep0, 0, MOJO_HANDLE_SIGNAL_WRITABLE); - remove_first.Add(&keep1, 0, MOJO_HANDLE_SIGNAL_WRITABLE); + remove_first.Add(&oneshot2, 0, false, MOJO_HANDLE_SIGNAL_WRITABLE); + remove_first.Add(&persistent0, 0, true, MOJO_HANDLE_SIGNAL_WRITABLE); + remove_first.Add(&persistent1, 0, true, MOJO_HANDLE_SIGNAL_WRITABLE); remove_first.OnStateChange( HandleSignalsState(MOJO_HANDLE_SIGNAL_NONE, MOJO_HANDLE_SIGNAL_WRITABLE), HandleSignalsState(MOJO_HANDLE_SIGNAL_WRITABLE, MOJO_HANDLE_SIGNAL_WRITABLE)); - EXPECT_EQ(keep0.awake_count, 1); - EXPECT_EQ(keep1.awake_count, 1); - EXPECT_EQ(remove2.awake_count, 1); + EXPECT_EQ(persistent0.awake_count, 1u); + EXPECT_EQ(persistent1.awake_count, 1u); + EXPECT_EQ(oneshot2.awake_count, 1u); remove_first.OnStateChange( HandleSignalsState(MOJO_HANDLE_SIGNAL_NONE, MOJO_HANDLE_SIGNAL_WRITABLE), HandleSignalsState(MOJO_HANDLE_SIGNAL_WRITABLE, MOJO_HANDLE_SIGNAL_WRITABLE)); - EXPECT_EQ(keep0.awake_count, 2); - EXPECT_EQ(keep1.awake_count, 2); - EXPECT_EQ(remove2.awake_count, 1); + EXPECT_EQ(persistent0.awake_count, 2u); + EXPECT_EQ(persistent1.awake_count, 2u); + EXPECT_EQ(oneshot2.awake_count, 1u); - remove_first.Remove(false, &keep0, 0); - remove_first.Remove(false, &keep1, 0); + remove_first.Remove(false, &persistent0, 0); + remove_first.Remove(false, &persistent1, 0); } } // namespace
diff --git a/mojo/edk/system/core_test_base.cc b/mojo/edk/system/core_test_base.cc index 30ae304..829519b 100644 --- a/mojo/edk/system/core_test_base.cc +++ b/mojo/edk/system/core_test_base.cc
@@ -172,7 +172,7 @@ MojoResult AddAwakableImplNoLock(Awakable* awakable, uint64_t /*context*/, - bool /*force*/, + bool /*persistent*/, MojoHandleSignals /*signals*/, HandleSignalsState* signals_state) override { info_->IncrementAddAwakableCallCount();
diff --git a/mojo/edk/system/core_unittest.cc b/mojo/edk/system/core_unittest.cc index e77cc98..901fcd5 100644 --- a/mojo/edk/system/core_unittest.cc +++ b/mojo/edk/system/core_unittest.cc
@@ -1953,8 +1953,8 @@ })); EXPECT_EQ(1u, info.GetAddedAwakableSize()); - EXPECT_FALSE(info.GetAddedAwakableAt(0)->Awake( - 0, Awakable::AwakeReason::UNSATISFIABLE, HandleSignalsState())); + info.GetAddedAwakableAt(0)->Awake(0, Awakable::AwakeReason::UNSATISFIABLE, + HandleSignalsState()); EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION, waiter.result); EXPECT_EQ(MOJO_RESULT_OK, core()->Close(h));
diff --git a/mojo/edk/system/data_pipe.cc b/mojo/edk/system/data_pipe.cc index 42c4d6e..9c54a09 100644 --- a/mojo/edk/system/data_pipe.cc +++ b/mojo/edk/system/data_pipe.cc
@@ -393,7 +393,7 @@ MojoResult DataPipe::ProducerAddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { MutexLocker locker(&mutex_); @@ -403,17 +403,17 @@ if (signals_state) *signals_state = producer_state; if (producer_state.satisfies(signals)) { - if (force) - producer_awakable_list_->Add(awakable, context, signals); + if (persistent) + producer_awakable_list_->Add(awakable, context, persistent, signals); return MOJO_RESULT_ALREADY_EXISTS; } if (!producer_state.can_satisfy(signals)) { - if (force) - producer_awakable_list_->Add(awakable, context, signals); + if (persistent) + producer_awakable_list_->Add(awakable, context, persistent, signals); return MOJO_RESULT_FAILED_PRECONDITION; } - producer_awakable_list_->Add(awakable, context, signals); + producer_awakable_list_->Add(awakable, context, persistent, signals); return MOJO_RESULT_OK; } @@ -607,7 +607,7 @@ MojoResult DataPipe::ConsumerAddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { MutexLocker locker(&mutex_); @@ -617,17 +617,17 @@ if (signals_state) *signals_state = consumer_state; if (consumer_state.satisfies(signals)) { - if (force) - consumer_awakable_list_->Add(awakable, context, signals); + if (persistent) + consumer_awakable_list_->Add(awakable, context, persistent, signals); return MOJO_RESULT_ALREADY_EXISTS; } if (!consumer_state.can_satisfy(signals)) { - if (force) - consumer_awakable_list_->Add(awakable, context, signals); + if (persistent) + consumer_awakable_list_->Add(awakable, context, persistent, signals); return MOJO_RESULT_FAILED_PRECONDITION; } - consumer_awakable_list_->Add(awakable, context, signals); + consumer_awakable_list_->Add(awakable, context, persistent, signals); return MOJO_RESULT_OK; } @@ -719,7 +719,7 @@ void DataPipe::ProducerCancelAllStateNoLock() { DCHECK(has_local_producer_no_lock()); if (producer_awakable_list_) - producer_awakable_list_->CancelAll(); + producer_awakable_list_->CancelAndRemoveAll(); // Not a bug, except possibly in "user" code. DVLOG_IF(2, producer_in_two_phase_write_no_lock()) << "Active two-phase write cancelled"; @@ -729,7 +729,7 @@ void DataPipe::ConsumerCancelAllStateNoLock() { DCHECK(has_local_consumer_no_lock()); if (consumer_awakable_list_) - consumer_awakable_list_->CancelAll(); + consumer_awakable_list_->CancelAndRemoveAll(); // Not a bug, except possibly in "user" code. DVLOG_IF(2, consumer_in_two_phase_read_no_lock()) << "Active two-phase read cancelled";
diff --git a/mojo/edk/system/data_pipe.h b/mojo/edk/system/data_pipe.h index 78fa471..2341d48 100644 --- a/mojo/edk/system/data_pipe.h +++ b/mojo/edk/system/data_pipe.h
@@ -116,7 +116,7 @@ HandleSignalsState ProducerGetHandleSignalsState(); MojoResult ProducerAddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state); void ProducerRemoveAwakable(bool match_context, @@ -153,7 +153,7 @@ HandleSignalsState ConsumerGetHandleSignalsState(); MojoResult ConsumerAddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state); void ConsumerRemoveAwakable(bool match_context,
diff --git a/mojo/edk/system/data_pipe_consumer_dispatcher.cc b/mojo/edk/system/data_pipe_consumer_dispatcher.cc index e7bf2f6..b70c05c 100644 --- a/mojo/edk/system/data_pipe_consumer_dispatcher.cc +++ b/mojo/edk/system/data_pipe_consumer_dispatcher.cc
@@ -195,11 +195,11 @@ MojoResult DataPipeConsumerDispatcher::AddAwakableImplNoLock( Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { mutex().AssertHeld(); - return data_pipe_->ConsumerAddAwakable(awakable, context, force, signals, + return data_pipe_->ConsumerAddAwakable(awakable, context, persistent, signals, signals_state); }
diff --git a/mojo/edk/system/data_pipe_consumer_dispatcher.h b/mojo/edk/system/data_pipe_consumer_dispatcher.h index de0a6a3..787d00f 100644 --- a/mojo/edk/system/data_pipe_consumer_dispatcher.h +++ b/mojo/edk/system/data_pipe_consumer_dispatcher.h
@@ -69,7 +69,7 @@ HandleSignalsState GetHandleSignalsStateImplNoLock() const override; MojoResult AddAwakableImplNoLock(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) override; void RemoveAwakableImplNoLock(bool match_context,
diff --git a/mojo/edk/system/data_pipe_impl_unittest.cc b/mojo/edk/system/data_pipe_impl_unittest.cc index 3f783ba..591598c 100644 --- a/mojo/edk/system/data_pipe_impl_unittest.cc +++ b/mojo/edk/system/data_pipe_impl_unittest.cc
@@ -141,10 +141,10 @@ } MojoResult ProducerAddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { - return dpp()->ProducerAddAwakable(awakable, context, force, signals, + return dpp()->ProducerAddAwakable(awakable, context, persistent, signals, signals_state); } void ProducerRemoveAwakable(bool match_context, @@ -184,10 +184,10 @@ } MojoResult ConsumerAddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { - return dpc()->ConsumerAddAwakable(awakable, context, force, signals, + return dpc()->ConsumerAddAwakable(awakable, context, persistent, signals, signals_state); } void ConsumerRemoveAwakable(bool match_context,
diff --git a/mojo/edk/system/data_pipe_producer_dispatcher.cc b/mojo/edk/system/data_pipe_producer_dispatcher.cc index c6696cd..2104a95 100644 --- a/mojo/edk/system/data_pipe_producer_dispatcher.cc +++ b/mojo/edk/system/data_pipe_producer_dispatcher.cc
@@ -170,11 +170,11 @@ MojoResult DataPipeProducerDispatcher::AddAwakableImplNoLock( Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { mutex().AssertHeld(); - return data_pipe_->ProducerAddAwakable(awakable, context, force, signals, + return data_pipe_->ProducerAddAwakable(awakable, context, persistent, signals, signals_state); }
diff --git a/mojo/edk/system/data_pipe_producer_dispatcher.h b/mojo/edk/system/data_pipe_producer_dispatcher.h index f2e9426..2f95747 100644 --- a/mojo/edk/system/data_pipe_producer_dispatcher.h +++ b/mojo/edk/system/data_pipe_producer_dispatcher.h
@@ -69,7 +69,7 @@ HandleSignalsState GetHandleSignalsStateImplNoLock() const override; MojoResult AddAwakableImplNoLock(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) override; void RemoveAwakableImplNoLock(bool match_context,
diff --git a/mojo/edk/system/dispatcher.cc b/mojo/edk/system/dispatcher.cc index 02304bd..dc62e05 100644 --- a/mojo/edk/system/dispatcher.cc +++ b/mojo/edk/system/dispatcher.cc
@@ -307,7 +307,7 @@ MojoResult Dispatcher::AddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { MutexLocker locker(&mutex_); @@ -317,7 +317,7 @@ return MOJO_RESULT_INVALID_ARGUMENT; } - return AddAwakableImplNoLock(awakable, context, force, signals, + return AddAwakableImplNoLock(awakable, context, persistent, signals, signals_state); } @@ -544,7 +544,7 @@ MojoResult Dispatcher::AddAwakableImplNoLock( Awakable* /*awakable*/, uint64_t /*context*/, - bool /*force*/, + bool /*persistent*/, MojoHandleSignals /*signals*/, HandleSignalsState* signals_state) { mutex_.AssertHeld();
diff --git a/mojo/edk/system/dispatcher.h b/mojo/edk/system/dispatcher.h index ea7596c..4ae4720 100644 --- a/mojo/edk/system/dispatcher.h +++ b/mojo/edk/system/dispatcher.h
@@ -193,9 +193,9 @@ // also be woken up when it becomes impossible for the object to ever satisfy // |signals| with a suitable error status. // - // If |force| is true, the awakable will be added even if |signals| is already - // satisfied or is never-satisfiable (if it is possible that some other change - // will cause it to become satisfiable again). + // If |persistent| is true, the awakable will be added even if |signals| is + // already satisfied or is never-satisfiable (if it is possible that some + // other change will cause it to become satisfiable again). // // If |signals_state| is non-null, |*signals_state| will be set to the current // handle signals state. @@ -209,14 +209,14 @@ // Returns: // - |MOJO_RESULT_OK| if the awakable was added; // - |MOJO_RESULT_ALREADY_EXISTS| if |signals| is already satisfied (if - // |force| is true, the awakable will still be added); + // |persistent| is true, the awakable will still be added); // - |MOJO_RESULT_INVALID_ARGUMENT| if the dispatcher has been closed; and // - |MOJO_RESULT_FAILED_PRECONDITION| if it is not (or no longer) possible - // that |signals| will ever be satisfied(if |force| is true, the awakable - // will still be added). + // that |signals| will ever be satisfied(if |persistent| is true, the + // awakable will still be added). MojoResult AddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state); // Removes an awakable from this dispatcher. This will remove all instances @@ -400,7 +400,7 @@ MOJO_SHARED_LOCKS_REQUIRED(mutex_); virtual MojoResult AddAwakableImplNoLock(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
diff --git a/mojo/edk/system/local_message_pipe_endpoint.cc b/mojo/edk/system/local_message_pipe_endpoint.cc index af59155..7708885 100644 --- a/mojo/edk/system/local_message_pipe_endpoint.cc +++ b/mojo/edk/system/local_message_pipe_endpoint.cc
@@ -65,7 +65,7 @@ void LocalMessagePipeEndpoint::CancelAllState() { DCHECK(is_open_); - awakable_list_.CancelAll(); + awakable_list_.CancelAndRemoveAll(); } MojoResult LocalMessagePipeEndpoint::ReadMessage( @@ -151,7 +151,7 @@ MojoResult LocalMessagePipeEndpoint::AddAwakable( Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { DCHECK(is_open_); @@ -160,17 +160,17 @@ if (signals_state) *signals_state = state; if (state.satisfies(signals)) { - if (force) - awakable_list_.Add(awakable, context, signals); + if (persistent) + awakable_list_.Add(awakable, context, persistent, signals); return MOJO_RESULT_ALREADY_EXISTS; } if (!state.can_satisfy(signals)) { - if (force) - awakable_list_.Add(awakable, context, signals); + if (persistent) + awakable_list_.Add(awakable, context, persistent, signals); return MOJO_RESULT_FAILED_PRECONDITION; } - awakable_list_.Add(awakable, context, signals); + awakable_list_.Add(awakable, context, persistent, signals); return MOJO_RESULT_OK; }
diff --git a/mojo/edk/system/local_message_pipe_endpoint.h b/mojo/edk/system/local_message_pipe_endpoint.h index dc2aeeb..a879f32 100644 --- a/mojo/edk/system/local_message_pipe_endpoint.h +++ b/mojo/edk/system/local_message_pipe_endpoint.h
@@ -39,7 +39,7 @@ HandleSignalsState GetHandleSignalsState() const override; MojoResult AddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) override; void RemoveAwakable(bool match_context,
diff --git a/mojo/edk/system/message_pipe.cc b/mojo/edk/system/message_pipe.cc index 9de43d6..309e22b 100644 --- a/mojo/edk/system/message_pipe.cc +++ b/mojo/edk/system/message_pipe.cc
@@ -192,7 +192,7 @@ MojoResult MessagePipe::AddAwakable(unsigned port, Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { DCHECK(port == 0 || port == 1); @@ -200,7 +200,7 @@ MutexLocker locker(&mutex_); DCHECK(endpoints_[port]); - return endpoints_[port]->AddAwakable(awakable, context, force, signals, + return endpoints_[port]->AddAwakable(awakable, context, persistent, signals, signals_state); }
diff --git a/mojo/edk/system/message_pipe.h b/mojo/edk/system/message_pipe.h index 547d83a..f0dc825 100644 --- a/mojo/edk/system/message_pipe.h +++ b/mojo/edk/system/message_pipe.h
@@ -102,7 +102,7 @@ MojoResult AddAwakable(unsigned port, Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state); void RemoveAwakable(unsigned port,
diff --git a/mojo/edk/system/message_pipe_dispatcher.cc b/mojo/edk/system/message_pipe_dispatcher.cc index 8c7994c..5722466 100644 --- a/mojo/edk/system/message_pipe_dispatcher.cc +++ b/mojo/edk/system/message_pipe_dispatcher.cc
@@ -203,12 +203,12 @@ MojoResult MessagePipeDispatcher::AddAwakableImplNoLock( Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { mutex().AssertHeld(); - return message_pipe_->AddAwakable(port_, awakable, context, force, signals, - signals_state); + return message_pipe_->AddAwakable(port_, awakable, context, persistent, + signals, signals_state); } void MessagePipeDispatcher::RemoveAwakableImplNoLock(
diff --git a/mojo/edk/system/message_pipe_dispatcher.h b/mojo/edk/system/message_pipe_dispatcher.h index 4f5c543..271410c 100644 --- a/mojo/edk/system/message_pipe_dispatcher.h +++ b/mojo/edk/system/message_pipe_dispatcher.h
@@ -98,7 +98,7 @@ HandleSignalsState GetHandleSignalsStateImplNoLock() const override; MojoResult AddAwakableImplNoLock(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) override; void RemoveAwakableImplNoLock(bool match_context,
diff --git a/mojo/edk/system/message_pipe_endpoint.cc b/mojo/edk/system/message_pipe_endpoint.cc index 42da701..5bcf782 100644 --- a/mojo/edk/system/message_pipe_endpoint.cc +++ b/mojo/edk/system/message_pipe_endpoint.cc
@@ -29,7 +29,7 @@ MojoResult MessagePipeEndpoint::AddAwakable(Awakable* /*awakable*/, uint64_t /*context*/, - bool /*force*/, + bool /*persistent*/, MojoHandleSignals /*signals*/, HandleSignalsState* signals_state) { NOTREACHED();
diff --git a/mojo/edk/system/message_pipe_endpoint.h b/mojo/edk/system/message_pipe_endpoint.h index ab12d89..e99a9f2 100644 --- a/mojo/edk/system/message_pipe_endpoint.h +++ b/mojo/edk/system/message_pipe_endpoint.h
@@ -67,7 +67,7 @@ virtual HandleSignalsState GetHandleSignalsState() const; virtual MojoResult AddAwakable(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state); virtual void RemoveAwakable(bool match_context,
diff --git a/mojo/edk/system/simple_dispatcher.cc b/mojo/edk/system/simple_dispatcher.cc index 25ae5ff..1d844bb 100644 --- a/mojo/edk/system/simple_dispatcher.cc +++ b/mojo/edk/system/simple_dispatcher.cc
@@ -22,13 +22,13 @@ void SimpleDispatcher::CancelAllStateNoLock() { mutex().AssertHeld(); - awakable_list_.CancelAll(); + awakable_list_.CancelAndRemoveAll(); } MojoResult SimpleDispatcher::AddAwakableImplNoLock( Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) { mutex().AssertHeld(); @@ -37,17 +37,17 @@ if (signals_state) *signals_state = state; if (state.satisfies(signals)) { - if (force) - awakable_list_.Add(awakable, context, signals); + if (persistent) + awakable_list_.Add(awakable, context, persistent, signals); return MOJO_RESULT_ALREADY_EXISTS; } if (!state.can_satisfy(signals)) { - if (force) - awakable_list_.Add(awakable, context, signals); + if (persistent) + awakable_list_.Add(awakable, context, persistent, signals); return MOJO_RESULT_FAILED_PRECONDITION; } - awakable_list_.Add(awakable, context, signals); + awakable_list_.Add(awakable, context, persistent, signals); return MOJO_RESULT_OK; }
diff --git a/mojo/edk/system/simple_dispatcher.h b/mojo/edk/system/simple_dispatcher.h index eecbf40..6626b4f 100644 --- a/mojo/edk/system/simple_dispatcher.h +++ b/mojo/edk/system/simple_dispatcher.h
@@ -34,7 +34,7 @@ void CancelAllStateNoLock() override; MojoResult AddAwakableImplNoLock(Awakable* awakable, uint64_t context, - bool force, + bool persistent, MojoHandleSignals signals, HandleSignalsState* signals_state) override; void RemoveAwakableImplNoLock(bool match_context,
diff --git a/mojo/edk/system/wait_set_dispatcher.cc b/mojo/edk/system/wait_set_dispatcher.cc index 4b5c7da..5af8b93 100644 --- a/mojo/edk/system/wait_set_dispatcher.cc +++ b/mojo/edk/system/wait_set_dispatcher.cc
@@ -197,12 +197,9 @@ if (result == MOJO_RESULT_ALREADY_EXISTS) { // It was added, but the wait condition is already satisfied. - AddPossiblyTriggeredNoLock(entry, Entry::TriggerState::POSSIBLY_SATISFIED); + AddPossiblyTriggeredNoLock(entry, Entry::TriggerState::SATISFIED); } else if (result == MOJO_RESULT_FAILED_PRECONDITION) { - // The condition is never-satisfiable. Leave a zombie entry (i.e., leave - // |dispatcher| null). - mutex().Unlock(); - return MOJO_RESULT_OK; + AddPossiblyTriggeredNoLock(entry, Entry::TriggerState::UNSATISFIABLE); } else if (result != MOJO_RESULT_OK) { size_t num_erased = entries_.erase(cookie); DCHECK_EQ(num_erased, 1u); @@ -260,7 +257,7 @@ return MOJO_RESULT_UNIMPLEMENTED; } -bool WaitSetDispatcher::Awake(uint64_t context, +void WaitSetDispatcher::Awake(uint64_t context, AwakeReason reason, const HandleSignalsState& signals_state) { MutexLocker locker(&mutex()); @@ -268,54 +265,51 @@ if (is_closed_no_lock()) { // See |CloseImplNoLock()|: This case may occur while we're unlocked in // |CloseImplNoLock()| (after that, we will have been removed from all the - // awakable lists, so |Awake()| should no longer be called). We may as well - // return false here, which will automatically remove ourselves from the - // awakable list (|CloseImplNoLock()| will call |RemoveAwakable()| anyway, - // but that's OK). - return false; + // awakable lists, so |Awake()| should no longer be called). + return; } auto it = entries_.find(context); DCHECK(it != entries_.end()); const auto& entry = it->second; + // We should only ever get at most one "closed" (cancelled). + DCHECK_NE(static_cast<int>(entry->trigger_state), + static_cast<int>(Entry::TriggerState::CLOSED)); switch (reason) { case AwakeReason::SATISFIED: - if (entry->trigger_state == Entry::TriggerState::NOT_TRIGGERED) { - AddPossiblyTriggeredNoLock(entry.get(), - Entry::TriggerState::POSSIBLY_SATISFIED); - } - return true; case AwakeReason::UNSATISFIABLE: - // Never satisfiable. - if (entry->trigger_state == Entry::TriggerState::NOT_TRIGGERED) { - AddPossiblyTriggeredNoLock(entry.get(), - Entry::TriggerState::NEVER_SATISFIABLE); - } else { - if (entry->trigger_state == Entry::TriggerState::POSSIBLY_SATISFIED) { - entry->trigger_state = Entry::TriggerState::NEVER_SATISFIABLE; + // We shouldn't see these since we're used as a persistent |Awakable|. + NOTREACHED(); + // Fall through. + case AwakeReason::CHANGED: + if (signals_state.satisfies(entry->signals)) { + if (entry->trigger_state == Entry::TriggerState::NOT_TRIGGERED) { + AddPossiblyTriggeredNoLock(entry.get(), + Entry::TriggerState::SATISFIED); } else { - // It's possible to get repeated "never satisfiable" triggers, but we - // shouldn't get anything after "closed". - DCHECK_NE(static_cast<int>(entry->trigger_state), - static_cast<int>(Entry::TriggerState::CLOSED)); + entry->trigger_state = Entry::TriggerState::SATISFIED; } + } else if (!signals_state.can_satisfy(entry->signals)) { + if (entry->trigger_state == Entry::TriggerState::NOT_TRIGGERED) { + AddPossiblyTriggeredNoLock(entry.get(), + Entry::TriggerState::UNSATISFIABLE); + } else { + entry->trigger_state = Entry::TriggerState::UNSATISFIABLE; + } + } else { + if (entry->trigger_state != Entry::TriggerState::NOT_TRIGGERED) + RemovePossiblyTriggeredNoLock(entry.get()); } - // Due to some action on some other thread, it may become satisfiable - // again, so continue to be awoken. - return true; + break; case AwakeReason::CANCELLED: if (entry->trigger_state == Entry::TriggerState::NOT_TRIGGERED) { AddPossiblyTriggeredNoLock(entry.get(), Entry::TriggerState::CLOSED); } else { - // We should only ever get at most one "closed". - DCHECK_NE(static_cast<int>(entry->trigger_state), - static_cast<int>(Entry::TriggerState::CLOSED)); entry->trigger_state = Entry::TriggerState::CLOSED; } entry->dispatcher = nullptr; - return false; + break; } - return false; } void WaitSetDispatcher::AddPossiblyTriggeredNoLock(
diff --git a/mojo/edk/system/wait_set_dispatcher.h b/mojo/edk/system/wait_set_dispatcher.h index d4f7cd5..4b7650d 100644 --- a/mojo/edk/system/wait_set_dispatcher.h +++ b/mojo/edk/system/wait_set_dispatcher.h
@@ -62,12 +62,7 @@ private: // Represents an entry in the wait set. struct Entry { - enum class TriggerState { - NOT_TRIGGERED, - POSSIBLY_SATISFIED, - NEVER_SATISFIABLE, - CLOSED - }; + enum class TriggerState { NOT_TRIGGERED, SATISFIED, UNSATISFIABLE, CLOSED }; Entry(MojoHandleSignals signals, uint64_t cookie); ~Entry(); @@ -113,7 +108,7 @@ UserPointer<uint32_t> max_results) override; // |Awakable| implementation: - bool Awake(uint64_t context, + void Awake(uint64_t context, AwakeReason reason, const HandleSignalsState& signals_state) override;
diff --git a/mojo/edk/system/waiter.cc b/mojo/edk/system/waiter.cc index e254b5c..3b67999 100644 --- a/mojo/edk/system/waiter.cc +++ b/mojo/edk/system/waiter.cc
@@ -92,13 +92,13 @@ return MojoResultForAwakeReason(awake_reason_); } -bool Waiter::Awake(uint64_t context, +void Waiter::Awake(uint64_t context, AwakeReason reason, const HandleSignalsState& signals_state) { MutexLocker locker(&mutex_); if (awoken_) - return true; + return; awoken_ = true; awake_reason_ = reason; @@ -107,7 +107,6 @@ cv_.Signal(); // |cv_.Wait()|/|cv_.WaitWithTimeout()| will return after |mutex_| is // released. - return true; } } // namespace system
diff --git a/mojo/edk/system/waiter.h b/mojo/edk/system/waiter.h index afe5b89..a2e97fc 100644 --- a/mojo/edk/system/waiter.h +++ b/mojo/edk/system/waiter.h
@@ -18,10 +18,15 @@ namespace mojo { namespace system { -// IMPORTANT (all-caps gets your attention, right?): |Waiter| methods are called -// under other locks, in particular, |Dispatcher::lock_|s, so |Waiter| methods -// must never call out to other objects (in particular, |Dispatcher|s). This -// class is thread-safe. +// An implementation of |Awakable| that is used for blocking waits. This should +// be used in a non-persistent way (i.e., |Awake()| should be called at most +// once by each source, and only for "leading edges"). +// +// IMPORTANT: |Waiter| methods are called under other locks, in particular, +// |Dispatcher::lock_|s, so |Waiter| methods must never call out to other +// objects (in particular, |Dispatcher|s). +// +// This class is thread-safe. class Waiter final : public Awakable { public: Waiter(); @@ -49,7 +54,7 @@ HandleSignalsState* signals_state); // |Awakable| implementation: - bool Awake(uint64_t context, + void Awake(uint64_t context, AwakeReason reason, const HandleSignalsState& signals_state) override;