[checkmk-commits] 5460 FIX Waiting without a wait condition immediately returns now

Module: check_mk Branch: master Commit: 8d32bc48d2a55f67720ead73e14a3dd15a54923f URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=8d32bc48d2a55f… Author: Sven Panne &lt;sp(a)mathias-kettner.de&gt; Date: Fri Nov 17 11:01:06 2017 +0100 5460 FIX Waiting without a wait condition immediately returns now Note: This change only affects users writing their own tools for the Livestatus interface, and only those tools which rely on an edge case. Up to now, Livestatus had a special obscure case: Using a WaitTrigger header in a query without a WaitCondition header resulted in waiting for the trigger exactly once. This is a misfeature, because it is effectively useless and doesn't do what one might expect: Due to spurious wakeups (https://en.wikipedia.org/wiki/Spurious_wakeup), waiting might return earlier basically at any time. To fix this behaviour, the special case has been removed, and a missing WaitCondition always has its expected semantics (always true) now. Note that this is consistent with a missing Filter header, which means "no filtering". Change-Id: I5e680ac54125d07a2dcd17c8d3aae9a89c0f0455 --- .werks/5460 | 23 +++++++++++++++++++++++ livestatus/src/Query.cc | 8 -------- livestatus/src/Query.h | 1 - 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/.werks/5460 b/.werks/5460 new file mode 100644 index 0000000..6f9cd0c --- /dev/null +++ b/.werks/5460 @@ -0,0 +1,23 @@ +Title: Waiting without a wait condition immediately returns now +Level: 1 +Component: livestatus +Compatible: incomp +Edition: cre +Version: 1.5.0i1 +Date: 1510912156 +Class: fix + +Note: This change only affects users writing their own tools for the +Livestatus interface, and only those tools which rely on an edge case. + +Up to now, Livestatus had a special obscure case: Using a WaitTrigger header +in a query without a WaitCondition header resulted in waiting for the +trigger exactly once. This is a misfeature, because it is effectively +useless and doesn't do what one might expect: Due to spurious wakeups +(https://en.wikipedia.org/wiki/Spurious_wakeup), waiting might return +earlier basically at any time. + +To fix this behaviour, the special case has been removed, and a missing +WaitCondition always has its expected semantics (always true) now. Note +that this is consistent with a missing Filter header, which means "no +filtering". diff --git a/livestatus/src/Query.cc b/livestatus/src/Query.cc index 1c75c4f..4759982 100644 --- a/livestatus/src/Query.cc +++ b/livestatus/src/Query.cc @@ -194,7 +194,6 @@ Query::Query(const std::list<std::string> &lines, Table *table, } _filter = std::make_unique<AndingFilter>(std::move(filters)); - _wait_conditions_empty = wait_conditions.empty(); _wait_condition = std::make_unique<AndingFilter>(std::move(wait_conditions)); } @@ -695,13 +694,6 @@ const std::vector<std::unique_ptr<Aggregator>> &Query::getAggregatorsFor( } void Query::doWait() { - // TODO(sp): Funny special case, do we really want/need this? - if (_wait_conditions_empty && _wait_trigger != nullptr) { - waitForTrigger(); - return; - } - - // Starting from here, it's basically a standard condition variable. while ( !_wait_condition->accepts(_wait_object, _auth_user, timezoneOffset())) { if (waitForTrigger() == std::cv_status::timeout) { diff --git a/livestatus/src/Query.h b/livestatus/src/Query.h index 6d080f1..f64f8e8 100644 --- a/livestatus/src/Query.h +++ b/livestatus/src/Query.h @@ -90,7 +90,6 @@ private: using FilterStack = std::vector<std::unique_ptr<Filter>>; std::unique_ptr<AndingFilter> _filter; contact *_auth_user; - bool _wait_conditions_empty; // TODO(sp): HACK, remove me... std::unique_ptr<AndingFilter> _wait_condition; std::chrono::milliseconds _wait_timeout; std::condition_variable *_wait_trigger;