From d8e08ecde97362f6b3117cbb68395fdd299027e2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 11 Jul 2019 14:52:30 -0400 Subject: DRTVWR-476: WIP: Untested preliminary implementation of LLCond. LLCond encapsulates the usage patterns required to properly use condition_variable. We also provide LLScalarCond, LLBoolCond and LLOneShotCond. --- indra/llcommon/llcond.h | 356 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 356 insertions(+) create mode 100644 indra/llcommon/llcond.h (limited to 'indra/llcommon/llcond.h') diff --git a/indra/llcommon/llcond.h b/indra/llcommon/llcond.h new file mode 100644 index 0000000000..adfeb27f82 --- /dev/null +++ b/indra/llcommon/llcond.h @@ -0,0 +1,356 @@ +/** + * @file llcond.h + * @author Nat Goodspeed + * @date 2019-07-10 + * @brief LLCond is a wrapper around condition_variable to encapsulate the + * obligatory condition_variable usage pattern. We also provide + * simplified versions LLScalarCond, LLBoolCond and LLOneShotCond. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLCOND_H) +#define LL_LLCOND_H + +#include +#include +#include + +/** + * LLCond encapsulates the pattern required to use a condition_variable. It + * bundles subject data, a mutex and a condition_variable: the three required + * data objects. It provides wait() methods analogous to condition_variable, + * but using the contained condition_variable and the contained mutex. It + * provides modify() methods accepting an invocable to safely modify the + * contained data and notify waiters. These methods implicitly perform the + * required locking. + * + * The generic LLCond template assumes that DATA might be a struct or class. + * For a scalar DATA type, consider LLScalarCond instead. For specifically + * bool, consider LLBoolCond. + * + * Use of boost::fibers::condition_variable makes LLCond work between + * coroutines as well as between threads. + */ +template +class LLCond +{ +private: + // This is the DATA controlled by the condition_variable. + DATA mData; + // condition_variable must be used in conjunction with a mutex. Use + // boost::fibers::mutex instead of std::mutex because the latter blocks + // the entire calling thread, whereas the former blocks only the current + // coroutine within the calling thread. Yet boost::fiber::mutex is safe to + // use across threads as well: it subsumes std::mutex functionality. + boost::fibers::mutex mMutex; + // Use boost::fibers::condition_variable for the same reason. + boost::fibers::condition_variable mCond; + +public: + /// LLCond can be explicitly initialized with a specific value for mData if + /// desired. + LLCond(DATA&& init=DATA()): + mData(init) + {} + + /// LLCond is move-only + LLCond(const LLCond&) = delete; + LLCond& operator=(const LLCond&) = delete; + + /// get() returns a const reference to the stored DATA. The only way to + /// get a non-const reference -- to modify the stored DATA -- is via + /// update_one() or update_all(). + const DATA& get() const { return mData; } + + /** + * Pass update_one() an invocable accepting non-const (DATA&). The + * invocable will presumably modify the referenced DATA. update_one() + * will lock the mutex, call the invocable and then call notify_one() on + * the condition_variable. + * + * For scalar DATA, it's simpler to use LLScalarCond::set_one(). Use + * update_one() when DATA is a struct or class. + */ + template + void update_one(MODIFY modify) + { + { // scope of lock can/should end before notify_one() + std::unique_lock lk(mMutex); + modify(mData); + } + mCond.notify_one(); + } + + /** + * Pass update_all() an invocable accepting non-const (DATA&). The + * invocable will presumably modify the referenced DATA. update_all() + * will lock the mutex, call the invocable and then call notify_all() on + * the condition_variable. + * + * For scalar DATA, it's simpler to use LLScalarCond::set_all(). Use + * update_all() when DATA is a struct or class. + */ + template + void update_all(MODIFY modify) + { + { // scope of lock can/should end before notify_all() + std::unique_lock lk(mMutex); + modify(mData); + } + mCond.notify_all(); + } + + /** + * Pass wait() 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() locks the mutex and, until the predicate returns true, calls + * wait() on the condition_variable. + */ + template + void wait(Pred pred) + { + std::unique_lock lk(mMutex); + // We must iterate explicitly since the predicate accepted by + // condition_variable::wait() requires a different signature: + // condition_variable::wait() calls its predicate with no arguments. + // Fortunately, the loop is straightforward. + // 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))) + { + mCond.wait(lk); + } + } + + /** + * 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; + } + + /** + * Pass wait_for() a chrono::duration, indicating how long we're willing + * to wait, 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_for() locks the mutex and, until the predicate returns true, calls + * wait_for() on the condition_variable. wait_for() returns false if + * condition_variable::wait_for() timed out without the predicate + * returning true. + */ + template + bool wait_for(const std::chrono::duration& timeout_duration, Pred pred) + { + // Instead of replicating wait_until() logic, convert duration to + // time_point and just call wait_until(). + // An implementation in which we repeatedly called + // condition_variable::wait_for() with our passed duration would be + // wrong! We'd keep pushing the timeout time farther and farther into + // the future. This way, we establish a definite timeout time and + // stick to it. + return wait_until(std::chrono::steady_clock::now() + timeout_duration, pred); + } +}; + +template +class LLScalarCond: public LLCond +{ + using super = LLCond; + +public: + /// LLScalarCond can be explicitly initialized with a specific value for + /// mData if desired. + LLCond(DATA&& init=DATA()): + super(init) + {} + + /// Pass set_one() a new value to which to update mData. set_one() will + /// lock the mutex, update mData and then call notify_one() on the + /// condition_variable. + void set_one(DATA&& value) + { + super::update_one([](DATA& data){ data = value; }); + } + + /// Pass set_all() a new value to which to update mData. set_all() will + /// lock the mutex, update mData and then call notify_all() on the + /// condition_variable. + void set_all(DATA&& value) + { + super::update_all([](DATA& data){ data = value; }); + } + + /** + * Pass wait_equal() a value for which to wait. wait_equal() locks the + * mutex and, until the stored DATA equals that value, calls wait() on the + * condition_variable. + */ + void wait_equal(const DATA& value) + { + super::wait([&value](const DATA& 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 DATA& value) + { + return super::wait_until(timeout_time, + [&value](const DATA& data){ return (data == 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 + * the mutex and, until the stored DATA equals that value, calls + * wait_for() on the condition_variable. wait_for_equal() returns false if + * condition_variable::wait_for() timed out without the stored DATA being + * equal to the passed value. + */ + template + bool wait_for_equal(const std::chrono::duration& timeout_duration, + const DATA& value) + { + return super::wait_for(timeout_duration, + [&value](const DATA& data){ return (data == value); }); + } + + /** + * Pass wait_unequal() a value from which to move away. wait_unequal() + * locks the mutex and, until the stored DATA no longer equals that value, + * calls wait() on the condition_variable. + */ + void wait_unequal(const DATA& value) + { + super::wait([&value](const DATA& 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 DATA& value) + { + return super::wait_until(timeout_time, + [&value](const DATA& data){ return (data != value); }); + } + + /** + * Pass wait_for_unequal() a chrono::duration, indicating how long we're + * willing to wait, and a value from which to move away. + * wait_for_unequal() locks the mutex and, until the stored DATA no longer + * equals that value, calls wait_for() on the condition_variable. + * wait_for_unequal() returns false if condition_variable::wait_for() + * timed out with the stored DATA still being equal to the passed value. + */ + template + bool wait_for_unequal(const std::chrono::duration& timeout_duration, + const DATA& value) + { + return super::wait_for(timeout_duration, + [&value](const DATA& data){ return (data != value); }); + } +}; + +/// Using bool as LLScalarCond's DATA seems like a particularly useful case +using LLBoolCond = LLScalarCond; + +// LLOneShotCond -- init false, set (and wait for) true? Or full suite? +class LLOneShotCond: public LLBoolCond +{ + using super = LLBoolCond; + +public: + /// The bool stored in LLOneShotCond is initially false + LLOneShotCond(): super(false) {} + + /// LLOneShotCond assumes that nullary set_one() means to set its bool true + void set_one(bool value=true) + { + super::set_one(value); + } + + /// LLOneShotCond assumes that nullary set_all() means to set its bool true + void set_all(bool value=true) + { + super::set_all(value); + } + + /** + * wait() locks the mutex and, until the stored bool is true, calls wait() + * on the condition_variable. + */ + void wait() + { + super::wait_equal(true); + } + + /** + * 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_equal(timeout_time, true); + } + + /** + * 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, + * calls wait_for() on the condition_variable. wait_for() returns false if + * condition_variable::wait_for() timed out without the stored bool being + * true. + */ + template + bool wait_for(const std::chrono::duration& timeout_duration) + { + return super::wait_for_equal(timeout_duration, true); + } +}; + +#endif /* ! defined(LL_LLCOND_H) */ -- cgit v1.2.3 From a3de18996f0c4aee37266c1a3d72c5cc9cadd6c1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Jul 2019 15:51:59 +0200 Subject: DRTVWR-476: Review response: support LLDate and llunits.h durations. Also introduce value_type typedef. --- indra/llcommon/llcond.h | 168 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 141 insertions(+), 27 deletions(-) (limited to 'indra/llcommon/llcond.h') diff --git a/indra/llcommon/llcond.h b/indra/llcommon/llcond.h index adfeb27f82..5ed9f10123 100644 --- a/indra/llcommon/llcond.h +++ b/indra/llcommon/llcond.h @@ -14,6 +14,8 @@ #if ! defined(LL_LLCOND_H) #define LL_LLCOND_H +#include "llunits.h" +#include "lldate.h" #include #include #include @@ -37,9 +39,12 @@ template class LLCond { +public: + typedef value_type DATA; + private: // This is the DATA controlled by the condition_variable. - DATA mData; + value_type mData; // condition_variable must be used in conjunction with a mutex. Use // boost::fibers::mutex instead of std::mutex because the latter blocks // the entire calling thread, whereas the former blocks only the current @@ -52,7 +57,7 @@ private: public: /// LLCond can be explicitly initialized with a specific value for mData if /// desired. - LLCond(DATA&& init=DATA()): + LLCond(value_type&& init=value_type()): mData(init) {} @@ -63,7 +68,7 @@ public: /// get() returns a const reference to the stored DATA. The only way to /// get a non-const reference -- to modify the stored DATA -- is via /// update_one() or update_all(). - const DATA& get() const { return mData; } + const value_type& get() const { return mData; } /** * Pass update_one() an invocable accepting non-const (DATA&). The @@ -122,7 +127,7 @@ public: // 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))) + while (! pred(const_cast(mData))) { mCond.wait(lk); } @@ -143,19 +148,29 @@ public: { std::unique_lock lk(mMutex); // see wait() for comments about this const_cast - while (! pred(const_cast(mData))) + 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 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 @@ -178,6 +193,24 @@ public: // stick to it. return wait_until(std::chrono::steady_clock::now() + timeout_duration, pred); } + + /** + * This wait_for() overload accepts F32Milliseconds as the duration. Any + * duration unit defined in llunits.h is implicitly convertible to + * F32Milliseconds. The semantics of this method are the same as the + * generic wait_for() method. + */ + template + bool wait_for(F32Milliseconds timeout_duration, Pred pred) + { + return wait_for(convert(timeout_duration), pred); + } + +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); }; template @@ -186,26 +219,32 @@ class LLScalarCond: public LLCond using super = LLCond; 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 /// mData if desired. - LLCond(DATA&& init=DATA()): + LLCond(value_type&& init=value_type()): super(init) {} /// Pass set_one() a new value to which to update mData. set_one() will /// lock the mutex, update mData and then call notify_one() on the /// condition_variable. - void set_one(DATA&& value) + void set_one(value_type&& value) { - super::update_one([](DATA& data){ data = value; }); + super::update_one([](value_type& data){ data = value; }); } /// Pass set_all() a new value to which to update mData. set_all() will /// lock the mutex, update mData and then call notify_all() on the /// condition_variable. - void set_all(DATA&& value) + void set_all(value_type&& value) { - super::update_all([](DATA& data){ data = value; }); + super::update_all([](value_type& data){ data = value; }); } /** @@ -213,9 +252,9 @@ public: * mutex and, until the stored DATA equals that value, calls wait() on the * condition_variable. */ - void wait_equal(const DATA& value) + void wait_equal(const value_type& value) { - super::wait([&value](const DATA& data){ return (data == value); }); + super::wait([&value](const value_type& data){ return (data == value); }); } /** @@ -228,10 +267,19 @@ public: */ template bool wait_until_equal(const std::chrono::time_point& timeout_time, - const DATA& value) + const value_type& value) { return super::wait_until(timeout_time, - [&value](const DATA& data){ return (data == value); }); + [&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); } /** @@ -244,10 +292,21 @@ public: */ template bool wait_for_equal(const std::chrono::duration& timeout_duration, - const DATA& value) + const value_type& value) { return super::wait_for(timeout_duration, - [&value](const DATA& data){ return (data == value); }); + [&value](const value_type& data){ return (data == value); }); + } + + /** + * This wait_for_equal() overload accepts F32Milliseconds as the duration. + * Any duration unit defined in llunits.h is implicitly convertible to + * F32Milliseconds. The semantics of this method are the same as the + * generic wait_for_equal() method. + */ + bool wait_for_equal(F32Milliseconds timeout_duration, const value_type& value) + { + return wait_for_equal(super::convert(timeout_duration), value); } /** @@ -255,9 +314,9 @@ public: * locks the mutex and, until the stored DATA no longer equals that value, * calls wait() on the condition_variable. */ - void wait_unequal(const DATA& value) + void wait_unequal(const value_type& value) { - super::wait([&value](const DATA& data){ return (data != value); }); + super::wait([&value](const value_type& data){ return (data != value); }); } /** @@ -270,10 +329,19 @@ public: */ template bool wait_until_unequal(const std::chrono::time_point& timeout_time, - const DATA& value) + const value_type& value) { return super::wait_until(timeout_time, - [&value](const DATA& data){ return (data != value); }); + [&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); } /** @@ -286,22 +354,48 @@ public: */ template bool wait_for_unequal(const std::chrono::duration& timeout_duration, - const DATA& value) + const value_type& value) { return super::wait_for(timeout_duration, - [&value](const DATA& data){ return (data != value); }); + [&value](const value_type& data){ return (data != value); }); + } + + /** + * This wait_for_unequal() overload accepts F32Milliseconds as the duration. + * Any duration unit defined in llunits.h is implicitly convertible to + * F32Milliseconds. The semantics of this method are the same as the + * generic wait_for_unequal() method. + */ + bool wait_for_unequal(F32Milliseconds timeout_duration, const value_type& value) + { + return wait_for_unequal(super::convert(timeout_duration), value); } + +protected: + using super::convert; }; /// Using bool as LLScalarCond's DATA seems like a particularly useful case using LLBoolCond = LLScalarCond; -// LLOneShotCond -- init false, set (and wait for) true? Or full suite? +/// LLOneShotCond -- init false, set (and wait for) true class LLOneShotCond: public LLBoolCond { using super = LLBoolCond; 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 LLOneShotCond(): super(false) {} @@ -323,7 +417,7 @@ public: */ void wait() { - super::wait_equal(true); + super::wait_unequal(false); } /** @@ -336,7 +430,16 @@ public: template bool wait_until(const std::chrono::time_point& timeout_time) { - return super::wait_until_equal(timeout_time, true); + 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)); } /** @@ -349,7 +452,18 @@ public: template bool wait_for(const std::chrono::duration& timeout_duration) { - return super::wait_for_equal(timeout_duration, true); + return super::wait_for_unequal(timeout_duration, false); + } + + /** + * This wait_for() overload accepts F32Milliseconds as the duration. + * Any duration unit defined in llunits.h is implicitly convertible to + * F32Milliseconds. The semantics of this method are the same as the + * generic wait_for() method. + */ + bool wait_for(F32Milliseconds timeout_duration) + { + return wait_for(super::convert(timeout_duration)); } }; -- cgit v1.2.3 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.2.3 From 72863b942829528a8a3af69d6e1e04a7ac139271 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Jul 2019 18:56:19 +0200 Subject: DRTVWR-476: Fix convert(F32Milliseconds) --- indra/llcommon/llcond.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llcond.h') diff --git a/indra/llcommon/llcond.h b/indra/llcommon/llcond.h index d18058cf62..8e7120582c 100644 --- a/indra/llcommon/llcond.h +++ b/indra/llcommon/llcond.h @@ -169,11 +169,11 @@ public: protected: // convert F32Milliseconds to a chrono::duration - std::chrono::milliseconds convert(F32Milliseconds) + std::chrono::milliseconds convert(F32Milliseconds duration) { // extract the F32 milliseconds from F32Milliseconds, construct // std::chrono::milliseconds from that value - return { timeout_duration.value() }; + return { duration.value() }; } private: -- cgit v1.2.3 From 3810145c1d8e21fc0819d71bbc42dcecced1f7e4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 18 Jul 2019 12:03:12 +0200 Subject: DRTVWR-476: Fix first round of compile errors. --- indra/llcommon/llcond.h | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) (limited to 'indra/llcommon/llcond.h') diff --git a/indra/llcommon/llcond.h b/indra/llcommon/llcond.h index 8e7120582c..b4289528de 100644 --- a/indra/llcommon/llcond.h +++ b/indra/llcommon/llcond.h @@ -39,7 +39,7 @@ template class LLCond { public: - typedef value_type DATA; + typedef DATA value_type; private: // This is the DATA controlled by the condition_variable. @@ -56,7 +56,7 @@ private: public: /// LLCond can be explicitly initialized with a specific value for mData if /// desired. - LLCond(value_type&& init=value_type()): + LLCond(const value_type& init=value_type()): mData(init) {} @@ -169,11 +169,16 @@ public: protected: // convert F32Milliseconds to a chrono::duration - std::chrono::milliseconds convert(F32Milliseconds duration) + auto convert(F32Milliseconds duration) { - // extract the F32 milliseconds from F32Milliseconds, construct - // std::chrono::milliseconds from that value - return { duration.value() }; + // std::chrono::milliseconds doesn't like to be constructed from a + // float (F32), rubbing our nose in the thought that + // std::chrono::duration::rep is probably integral. Therefore + // converting F32Milliseconds to std::chrono::milliseconds would lose + // precision. Use std::chrono::microseconds instead. Extract the F32 + // milliseconds from F32Milliseconds, scale to microseconds, construct + // std::chrono::microseconds from that value. + return std::chrono::microseconds{ std::chrono::microseconds::rep(duration.value() * 1000) }; } private: @@ -224,31 +229,31 @@ class LLScalarCond: public LLCond using super = LLCond; public: - using super::value_type; + using typename super::value_type; using super::get; using super::wait; using super::wait_for; /// LLScalarCond can be explicitly initialized with a specific value for /// mData if desired. - LLCond(value_type&& init=value_type()): + LLScalarCond(const value_type& init=value_type()): super(init) {} /// Pass set_one() a new value to which to update mData. set_one() will /// lock the mutex, update mData and then call notify_one() on the /// condition_variable. - void set_one(value_type&& value) + void set_one(const value_type& value) { - super::update_one([](value_type& data){ data = value; }); + super::update_one([&value](value_type& data){ data = value; }); } /// Pass set_all() a new value to which to update mData. set_all() will /// lock the mutex, update mData and then call notify_all() on the /// condition_variable. - void set_all(value_type&& value) + void set_all(const value_type& value) { - super::update_all([](value_type& data){ data = value; }); + super::update_all([&value](value_type& data){ data = value; }); } /** @@ -338,7 +343,7 @@ class LLOneShotCond: public LLBoolCond using super = LLBoolCond; public: - using super::value_type; + using typename super::value_type; using super::get; using super::wait; using super::wait_for; -- cgit v1.2.3 From 98dfba0d2f24aeb92e023df9d48b23fef8253024 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 14 May 2020 14:51:52 -0400 Subject: DRTVWR-476: Wrap boost::fibers::mutex et al. with LLCoros aliases. Specifically: LLCoros::Mutex means boost::fibers::mutex LLCoros::LockType means std::unique_lock LLCoros::ConditionVariable means boost::fibers::condition_variable LLCoros::cv_status means boost::fibers::cv_status So as not to drag in all of boost::fibers::mutex.hpp or condition_variable.hpp for each consumer of llcoros.h, instead #define LLCOROS_MUTEX_HEADER and LLCOROS_CONDVAR_HEADER. Those who need them can #include the relevant macro. Update llcond.h and llthreadsafequeue.h accordingly. --- indra/llcommon/llcond.h | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'indra/llcommon/llcond.h') diff --git a/indra/llcommon/llcond.h b/indra/llcommon/llcond.h index b4289528de..e31b67d893 100644 --- a/indra/llcommon/llcond.h +++ b/indra/llcommon/llcond.h @@ -15,8 +15,9 @@ #define LL_LLCOND_H #include "llunits.h" -#include -#include +#include "llcoros.h" +#include LLCOROS_MUTEX_HEADER +#include "mutex.h" #include /** @@ -32,7 +33,7 @@ * For a scalar DATA type, consider LLScalarCond instead. For specifically * bool, consider LLBoolCond. * - * Use of boost::fibers::condition_variable makes LLCond work between + * Use of LLCoros::ConditionVariable makes LLCond work between * coroutines as well as between threads. */ template @@ -45,13 +46,13 @@ private: // This is the DATA controlled by the condition_variable. value_type mData; // condition_variable must be used in conjunction with a mutex. Use - // boost::fibers::mutex instead of std::mutex because the latter blocks + // LLCoros::Mutex instead of std::mutex because the latter blocks // the entire calling thread, whereas the former blocks only the current - // coroutine within the calling thread. Yet boost::fiber::mutex is safe to + // coroutine within the calling thread. Yet LLCoros::Mutex is safe to // use across threads as well: it subsumes std::mutex functionality. - boost::fibers::mutex mMutex; - // Use boost::fibers::condition_variable for the same reason. - boost::fibers::condition_variable mCond; + LLCoros::Mutex mMutex; + // Use LLCoros::ConditionVariable for the same reason. + LLCoros::ConditionVariable mCond; public: /// LLCond can be explicitly initialized with a specific value for mData if @@ -82,7 +83,7 @@ public: void update_one(MODIFY modify) { { // scope of lock can/should end before notify_one() - std::unique_lock lk(mMutex); + LLCoros::LockType lk(mMutex); modify(mData); } mCond.notify_one(); @@ -101,7 +102,7 @@ public: void update_all(MODIFY modify) { { // scope of lock can/should end before notify_all() - std::unique_lock lk(mMutex); + LLCoros::LockType lk(mMutex); modify(mData); } mCond.notify_all(); @@ -117,7 +118,7 @@ public: template void wait(Pred pred) { - std::unique_lock lk(mMutex); + LLCoros::LockType lk(mMutex); // We must iterate explicitly since the predicate accepted by // condition_variable::wait() requires a different signature: // condition_variable::wait() calls its predicate with no arguments. @@ -204,14 +205,14 @@ private: template bool wait_until(const std::chrono::time_point& timeout_time, Pred pred) { - std::unique_lock lk(mMutex); + LLCoros::LockType 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)) + if (LLCoros::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 -- cgit v1.2.3