From 7e2cc57d61d09ca106243d51494aef025c8729fa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Jul 2019 18:49:31 +0200 Subject: DRTVWR-476: Review response: remove wait_until() methods and LLDate. --- indra/llcommon/llcond.h | 165 ++++++++++++++---------------------------------- 1 file changed, 47 insertions(+), 118 deletions(-) (limited to 'indra/llcommon/llcond.h') diff --git a/indra/llcommon/llcond.h b/indra/llcommon/llcond.h index 5ed9f10123..d18058cf62 100644 --- a/indra/llcommon/llcond.h +++ b/indra/llcommon/llcond.h @@ -15,7 +15,6 @@ #define LL_LLCOND_H #include "llunits.h" -#include "lldate.h" #include #include #include @@ -133,44 +132,6 @@ public: } } - /** - * Pass wait_until() a chrono::time_point, indicating the time at which we - * should stop waiting, and a predicate accepting (const DATA&), returning - * bool. The predicate returns true when the condition for which it is - * waiting has been satisfied, presumably determined by examining the - * referenced DATA. wait_until() locks the mutex and, until the predicate - * returns true, calls wait_until() on the condition_variable. - * wait_until() returns false if condition_variable::wait_until() timed - * out without the predicate returning true. - */ - template - bool wait_until(const std::chrono::time_point& timeout_time, Pred pred) - { - std::unique_lock lk(mMutex); - // see wait() for comments about this const_cast - while (! pred(const_cast(mData))) - { - if (boost::fibers::cv_status::timeout == mCond.wait_until(lk, timeout_time)) - { - // It's possible that wait_until() timed out AND the predicate - // became true more or less simultaneously. Even though - // wait_until() timed out, check the predicate one more time. - return pred(const_cast(mData)); - } - } - return true; - } - - /** - * This wait_until() overload accepts LLDate as the time_point. Its - * semantics are the same as the generic wait_until() method. - */ - template - bool wait_until(const LLDate& timeout_time, Pred pred) - { - return wait_until(convert(timeout_time), pred); - } - /** * Pass wait_for() a chrono::duration, indicating how long we're willing * to wait, and a predicate accepting (const DATA&), returning bool. The @@ -207,10 +168,54 @@ public: } protected: - // convert LLDate to a chrono::time_point - std::chrono::system_clock::time_point convert(const LLDate&); // convert F32Milliseconds to a chrono::duration - std::chrono::milliseconds convert(F32Milliseconds); + std::chrono::milliseconds convert(F32Milliseconds) + { + // extract the F32 milliseconds from F32Milliseconds, construct + // std::chrono::milliseconds from that value + return { timeout_duration.value() }; + } + +private: + /** + * Pass wait_until() a chrono::time_point, indicating the time at which we + * should stop waiting, and a predicate accepting (const DATA&), returning + * bool. The predicate returns true when the condition for which it is + * waiting has been satisfied, presumably determined by examining the + * referenced DATA. wait_until() locks the mutex and, until the predicate + * returns true, calls wait_until() on the condition_variable. + * wait_until() returns false if condition_variable::wait_until() timed + * out without the predicate returning true. + * + * Originally this class and its subclasses published wait_until() methods + * corresponding to each wait_for() method. But that raised all sorts of + * fascinating questions about the time zone of the passed time_point: + * local time? server time? UTC? The bottom line is that for LLCond + * timeout purposes, we really shouldn't have to care -- timeout duration + * is all we need. This private method remains because it's the simplest + * way to support iteratively waiting across spurious wakeups while + * honoring a fixed timeout. + */ + template + bool wait_until(const std::chrono::time_point& timeout_time, Pred pred) + { + std::unique_lock lk(mMutex); + // We advise the caller to pass a predicate accepting (const DATA&). + // But what if they instead pass a predicate accepting non-const + // (DATA&)? Such a predicate could modify mData, which would be Bad. + // Forbid that. + while (! pred(const_cast(mData))) + { + if (boost::fibers::cv_status::timeout == mCond.wait_until(lk, timeout_time)) + { + // It's possible that wait_until() timed out AND the predicate + // became true more or less simultaneously. Even though + // wait_until() timed out, check the predicate one more time. + return pred(const_cast(mData)); + } + } + return true; + } }; template @@ -222,7 +227,6 @@ public: using super::value_type; using super::get; using super::wait; - using super::wait_until; using super::wait_for; /// LLScalarCond can be explicitly initialized with a specific value for @@ -257,31 +261,6 @@ public: super::wait([&value](const value_type& data){ return (data == value); }); } - /** - * Pass wait_until_equal() a chrono::time_point, indicating the time at - * which we should stop waiting, and a value for which to wait. - * wait_until_equal() locks the mutex and, until the stored DATA equals - * that value, calls wait_until() on the condition_variable. - * wait_until_equal() returns false if condition_variable::wait_until() - * timed out without the stored DATA being equal to the passed value. - */ - template - bool wait_until_equal(const std::chrono::time_point& timeout_time, - const value_type& value) - { - return super::wait_until(timeout_time, - [&value](const value_type& data){ return (data == value); }); - } - - /** - * This wait_until_equal() overload accepts LLDate as the time_point. Its - * semantics are the same as the generic wait_until_equal() method. - */ - bool wait_until_equal(const LLDate& timeout_time, const value_type& value) - { - return wait_until_equal(super::convert(timeout_time), value); - } - /** * Pass wait_for_equal() a chrono::duration, indicating how long we're * willing to wait, and a value for which to wait. wait_for_equal() locks @@ -319,31 +298,6 @@ public: super::wait([&value](const value_type& data){ return (data != value); }); } - /** - * Pass wait_until_unequal() a chrono::time_point, indicating the time at - * which we should stop waiting, and a value from which to move away. - * wait_until_unequal() locks the mutex and, until the stored DATA no - * longer equals that value, calls wait_until() on the condition_variable. - * wait_until_unequal() returns false if condition_variable::wait_until() - * timed out with the stored DATA still being equal to the passed value. - */ - template - bool wait_until_unequal(const std::chrono::time_point& timeout_time, - const value_type& value) - { - return super::wait_until(timeout_time, - [&value](const value_type& data){ return (data != value); }); - } - - /** - * This wait_until_unequal() overload accepts LLDate as the time_point. - * Its semantics are the same as the generic wait_until_unequal() method. - */ - bool wait_until_unequal(const LLDate& timeout_time, const value_type& value) - { - return wait_until_unequal(super::convert(timeout_time), value); - } - /** * Pass wait_for_unequal() a chrono::duration, indicating how long we're * willing to wait, and a value from which to move away. @@ -387,13 +341,10 @@ public: using super::value_type; using super::get; using super::wait; - using super::wait_until; using super::wait_for; using super::wait_equal; - using super::wait_until_equal; using super::wait_for_equal; using super::wait_unequal; - using super::wait_until_unequal; using super::wait_for_unequal; /// The bool stored in LLOneShotCond is initially false @@ -420,28 +371,6 @@ public: super::wait_unequal(false); } - /** - * Pass wait_until() a chrono::time_point, indicating the time at which we - * should stop waiting. wait_until() locks the mutex and, until the stored - * bool is true, calls wait_until() on the condition_variable. - * wait_until() returns false if condition_variable::wait_until() timed - * out without the stored bool being true. - */ - template - bool wait_until(const std::chrono::time_point& timeout_time) - { - return super::wait_until_unequal(timeout_time, false); - } - - /** - * This wait_until() overload accepts LLDate as the time_point. - * Its semantics are the same as the generic wait_until() method. - */ - bool wait_until(const LLDate& timeout_time) - { - return wait_until(super::convert(timeout_time)); - } - /** * Pass wait_for() a chrono::duration, indicating how long we're willing * to wait. wait_for() locks the mutex and, until the stored bool is true, -- cgit v1.3