From 078ae69d0845ce69ea2a6c2e18ff5f6a031a8614 Mon Sep 17 00:00:00 2001 From: Maxim Pimenov Date: Tue, 22 Oct 2019 12:27:02 +0300 Subject: [PATCH] [base] Added an option for setting a deadline to Cancellable. Two things that I'm not fond of in this commit: 1. mutable Status 2. It's unclear what to do if a call to Cancel() arrives after the deadline has fired and has been checked by a client. In this implementation, a client may call IsCancelled() twice and observe DeadlineExceeded after the first call and CancelCalled after the second. If it's the other way around (first cancel was called and then the deadline fired), the client will observe CancelCalled both times, i.e. an explicit cancelling is deemed more important. --- base/CMakeLists.txt | 1 + base/base_tests/CMakeLists.txt | 1 + base/base_tests/cancellable_tests.cpp | 81 +++++++++++++++++++++++ base/cancellable.cpp | 64 ++++++++++++++++++ base/cancellable.hpp | 42 ++++++++++-- xcode/base/base.xcodeproj/project.pbxproj | 16 +++-- 6 files changed, 195 insertions(+), 10 deletions(-) create mode 100644 base/base_tests/cancellable_tests.cpp create mode 100644 base/cancellable.cpp diff --git a/base/CMakeLists.txt b/base/CMakeLists.txt index eb86eba0ab..79f679ca09 100644 --- a/base/CMakeLists.txt +++ b/base/CMakeLists.txt @@ -14,6 +14,7 @@ set( bwt.cpp bwt.hpp cache.hpp + cancellable.cpp cancellable.hpp checked_cast.hpp clustering_map.hpp diff --git a/base/base_tests/CMakeLists.txt b/base/base_tests/CMakeLists.txt index 628b4bea9c..37d881312f 100644 --- a/base/base_tests/CMakeLists.txt +++ b/base/base_tests/CMakeLists.txt @@ -11,6 +11,7 @@ set( buffer_vector_test.cpp bwt_tests.cpp cache_test.cpp + cancellable_tests.cpp clustering_map_tests.cpp collection_cast_test.cpp condition_test.cpp diff --git a/base/base_tests/cancellable_tests.cpp b/base/base_tests/cancellable_tests.cpp new file mode 100644 index 0000000000..b3acdf6060 --- /dev/null +++ b/base/base_tests/cancellable_tests.cpp @@ -0,0 +1,81 @@ +#include "testing/testing.hpp" + +#include "base/cancellable.hpp" +#include "base/math.hpp" +#include "base/thread.hpp" + +#include +#include +#include + +using namespace std; + +namespace base +{ +UNIT_TEST(Cancellable_Smoke) +{ + Cancellable cancellable; + + promise promise; + auto future = promise.get_future(); + + double x = 0.123; + + auto const fn = [&] { + for (size_t it = 0;; it++) + { + if (it > 100 && cancellable.IsCancelled()) + break; + + x = cos(x); + } + + promise.set_value(); + }; + + threads::SimpleThread thread(fn); + cancellable.Cancel(); + future.wait(); + thread.join(); + + TEST(cancellable.IsCancelled(), ()); + TEST_EQUAL(cancellable.CancellationStatus(), Cancellable::Status::CancelCalled, ()); + TEST(base::AlmostEqualAbs(x, 0.739, 1e-3), ()); +} + +UNIT_TEST(Cancellable_Deadline) +{ + Cancellable cancellable; + chrono::steady_clock::duration kTimeout = chrono::milliseconds(20); + cancellable.SetDeadline(chrono::steady_clock::now() + kTimeout); + + promise promise; + auto future = promise.get_future(); + + double x = 0.123; + + auto const fn = [&] { + for (size_t it = 0;; it++) + { + if (cancellable.IsCancelled()) + break; + + x = cos(x); + } + + promise.set_value(); + }; + + threads::SimpleThread thread(fn); + future.wait(); + thread.join(); + + TEST(cancellable.IsCancelled(), ()); + TEST_EQUAL(cancellable.CancellationStatus(), Cancellable::Status::DeadlineExceeded, ()); + TEST(base::AlmostEqualAbs(x, 0.739, 1e-3), ()); + + cancellable.Cancel(); + TEST(cancellable.IsCancelled(), ()); + TEST_EQUAL(cancellable.CancellationStatus(), Cancellable::Status::CancelCalled, ()); +} +} // namespace base diff --git a/base/cancellable.cpp b/base/cancellable.cpp new file mode 100644 index 0000000000..a4e8e4d429 --- /dev/null +++ b/base/cancellable.cpp @@ -0,0 +1,64 @@ +#include "base/cancellable.hpp" + +#include "base/assert.hpp" + +namespace base +{ +void Cancellable::Reset() +{ + std::lock_guard lock(m_mutex); + + m_status = Status::Active; + m_deadline = {}; +} + +void Cancellable::Cancel() +{ + std::lock_guard lock(m_mutex); + + m_status = Status::CancelCalled; +} + +void Cancellable::SetDeadline(std::chrono::steady_clock::time_point const & deadline) +{ + std::lock_guard lock(m_mutex); + + m_deadline = deadline; + CheckDeadline(); +} + +bool Cancellable::IsCancelled() const +{ + std::lock_guard lock(m_mutex); + + CheckDeadline(); + return m_status != Status::Active; +} + +Cancellable::Status Cancellable::CancellationStatus() const +{ + std::lock_guard lock(m_mutex); + + return m_status; +} + +void Cancellable::CheckDeadline() const +{ + if (m_status == Status::Active && + m_deadline && *m_deadline < std::chrono::steady_clock::now()) + { + m_status = Status::DeadlineExceeded; + } +} + +std::string DebugPrint(Cancellable::Status status) +{ + switch (status) + { + case Cancellable::Status::Active: return "Active"; + case Cancellable::Status::CancelCalled: return "CancelCalled"; + case Cancellable::Status::DeadlineExceeded: return "DeadlineExceeded"; + } + UNREACHABLE(); +} +} // namespace base diff --git a/base/cancellable.hpp b/base/cancellable.hpp index a43713e1f2..ba333f1012 100644 --- a/base/cancellable.hpp +++ b/base/cancellable.hpp @@ -1,6 +1,11 @@ #pragma once #include +#include +#include +#include + +#include namespace base { @@ -9,20 +14,45 @@ namespace base class Cancellable { public: - Cancellable() : m_cancelled(false) {} + enum class Status + { + Active, + CancelCalled, + DeadlineExceeded, + }; + + Cancellable() = default; virtual ~Cancellable() {} // Marks current activity as not cancelled. - virtual void Reset() { m_cancelled = false; } + // Resets the deadline if it was present. + virtual void Reset(); // Marks current activity as cancelled. - virtual void Cancel() { m_cancelled = true; } + virtual void Cancel(); - // Returns true iff current activity has been cancelled. - virtual bool IsCancelled() const { return m_cancelled; } + // Sets a deadline after which the activity is cancelled. + virtual void SetDeadline(std::chrono::steady_clock::time_point const & t); + + // Returns true iff current activity has been cancelled (either by the call + // to Cancel or by timeout). + virtual bool IsCancelled() const; + + // Returns the latest known status. Does not update the status before returning, + // i.e. may miss crossing the deadline if the latest update was too long ago. + virtual Status CancellationStatus() const; private: - std::atomic m_cancelled; + // Checks whether |m_deadline| has exceeded. Must be called with |m_mutex| locked. + void CheckDeadline() const; + + mutable std::mutex m_mutex; + + mutable Status m_status = Status::Active; + + boost::optional m_deadline; }; + +std::string DebugPrint(Cancellable::Status status); } // namespace base diff --git a/xcode/base/base.xcodeproj/project.pbxproj b/xcode/base/base.xcodeproj/project.pbxproj index 8f145f4533..8bda1c48fc 100644 --- a/xcode/base/base.xcodeproj/project.pbxproj +++ b/xcode/base/base.xcodeproj/project.pbxproj @@ -16,6 +16,7 @@ 3446C67D1DDCAA4900146687 /* uni_string_dfa_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3446C6791DDCAA2500146687 /* uni_string_dfa_test.cpp */; }; 3446C6821DDCAA7400146687 /* newtype_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3446C67E1DDCAA6E00146687 /* newtype_test.cpp */; }; 3446C6831DDCAA7800146687 /* ref_counted_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3446C67F1DDCAA6E00146687 /* ref_counted_tests.cpp */; }; + 3901B745235F02B3006ABD43 /* cancellable.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3901B744235F02B2006ABD43 /* cancellable.cpp */; }; 390F1C0D2294298E00EA58E3 /* file_name_utils.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 390F1C0B2294298E00EA58E3 /* file_name_utils.hpp */; }; 390F1C0E2294298E00EA58E3 /* file_name_utils.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 390F1C0C2294298E00EA58E3 /* file_name_utils.cpp */; }; 390F1C112294299E00EA58E3 /* file_name_utils_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 390F1C0F2294299A00EA58E3 /* file_name_utils_tests.cpp */; }; @@ -37,9 +38,10 @@ 3917FA69211E010400937DF4 /* fifo_cache_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 564BB446206E8A4D00BDD211 /* fifo_cache_test.cpp */; }; 3917FA6A211E010400937DF4 /* control_flow_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 39B89C391FD1898A001104AF /* control_flow_tests.cpp */; }; 3917FA6B211E010400937DF4 /* small_set_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 67E40EC71E4DC0D500A6D200 /* small_set_test.cpp */; }; - 395784D02303034100F2CC07 /* thread_safe_queue_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 395784CE2303034000F2CC07 /* thread_safe_queue_tests.cpp */; }; - 395784D12303034100F2CC07 /* url_helpers_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 395784CF2303034000F2CC07 /* url_helpers_tests.cpp */; }; 395FEB3521492F350036395C /* stl_helpers_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 395FEB3321492F320036395C /* stl_helpers_tests.cpp */; }; + 3975D20B235F2738004D84D3 /* cancellable_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3975D20A235F2738004D84D3 /* cancellable_tests.cpp */; }; + 3975D20C235F2761004D84D3 /* thread_safe_queue_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 395784CE2303034000F2CC07 /* thread_safe_queue_tests.cpp */; }; + 3975D20D235F2761004D84D3 /* url_helpers_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 395784CF2303034000F2CC07 /* url_helpers_tests.cpp */; }; 397DC50D22BB8EBF007126DB /* bidirectional_map.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 397DC50B22BB8EBF007126DB /* bidirectional_map.hpp */; }; 397DC50E22BB8EBF007126DB /* non_intersecting_intervals.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 397DC50C22BB8EBF007126DB /* non_intersecting_intervals.hpp */; }; 397DC51122BB8ED2007126DB /* bidirectional_map_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 397DC50F22BB8ECD007126DB /* bidirectional_map_tests.cpp */; }; @@ -172,6 +174,7 @@ 3446C67F1DDCAA6E00146687 /* ref_counted_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ref_counted_tests.cpp; sourceTree = ""; }; 34BA2D6A1DBE169E00FAB345 /* common-debug.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = "common-debug.xcconfig"; path = "../common-debug.xcconfig"; sourceTree = ""; }; 34BA2D6B1DBE169E00FAB345 /* common-release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = "common-release.xcconfig"; path = "../common-release.xcconfig"; sourceTree = ""; }; + 3901B744235F02B2006ABD43 /* cancellable.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = cancellable.cpp; sourceTree = ""; }; 390F1C0B2294298E00EA58E3 /* file_name_utils.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = file_name_utils.hpp; sourceTree = ""; }; 390F1C0C2294298E00EA58E3 /* file_name_utils.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = file_name_utils.cpp; sourceTree = ""; }; 390F1C0F2294299A00EA58E3 /* file_name_utils_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = file_name_utils_tests.cpp; sourceTree = ""; }; @@ -186,6 +189,7 @@ 395784CE2303034000F2CC07 /* thread_safe_queue_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = thread_safe_queue_tests.cpp; sourceTree = ""; }; 395784CF2303034000F2CC07 /* url_helpers_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = url_helpers_tests.cpp; sourceTree = ""; }; 395FEB3321492F320036395C /* stl_helpers_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = stl_helpers_tests.cpp; sourceTree = ""; }; + 3975D20A235F2738004D84D3 /* cancellable_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = cancellable_tests.cpp; sourceTree = ""; }; 397DC50B22BB8EBF007126DB /* bidirectional_map.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = bidirectional_map.hpp; sourceTree = ""; }; 397DC50C22BB8EBF007126DB /* non_intersecting_intervals.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = non_intersecting_intervals.hpp; sourceTree = ""; }; 397DC50F22BB8ECD007126DB /* bidirectional_map_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = bidirectional_map_tests.cpp; sourceTree = ""; }; @@ -344,6 +348,7 @@ 39FD26C71CC659D200AFF551 /* base_tests */ = { isa = PBXGroup; children = ( + 3975D20A235F2738004D84D3 /* cancellable_tests.cpp */, 395784CE2303034000F2CC07 /* thread_safe_queue_tests.cpp */, 395784CF2303034000F2CC07 /* url_helpers_tests.cpp */, 397DC50F22BB8ECD007126DB /* bidirectional_map_tests.cpp */, @@ -420,6 +425,7 @@ 675341791A3F57BF00A0A8C3 /* base */ = { isa = PBXGroup; children = ( + 3901B744235F02B2006ABD43 /* cancellable.cpp */, 3989E363230302EA00D63F84 /* thread_safe_queue.hpp */, 397DC50B22BB8EBF007126DB /* bidirectional_map.hpp */, 397DC50C22BB8EBF007126DB /* non_intersecting_intervals.hpp */, @@ -732,9 +738,12 @@ 3917FA6A211E010400937DF4 /* control_flow_tests.cpp in Sources */, 39FD27321CC65AD000AFF551 /* string_format_test.cpp in Sources */, 39FD272A1CC65AD000AFF551 /* mem_trie_test.cpp in Sources */, + 3975D20D235F2761004D84D3 /* url_helpers_tests.cpp in Sources */, 395FEB3521492F350036395C /* stl_helpers_tests.cpp in Sources */, + 3975D20C235F2761004D84D3 /* thread_safe_queue_tests.cpp in Sources */, 3446C67C1DDCAA4600146687 /* levenshtein_dfa_test.cpp in Sources */, 3917FA69211E010400937DF4 /* fifo_cache_test.cpp in Sources */, + 3975D20B235F2738004D84D3 /* cancellable_tests.cpp in Sources */, 39FD27381CC65AD000AFF551 /* timegm_test.cpp in Sources */, 39FD272C1CC65AD000AFF551 /* range_iterator_test.cpp in Sources */, 39FD27261CC65AD000AFF551 /* containers_test.cpp in Sources */, @@ -784,6 +793,7 @@ 675341DA1A3F57E400A0A8C3 /* exception.cpp in Sources */, 3D74EF111F8B902C0081202C /* suffix_array.cpp in Sources */, 3917FA57211E009700937DF4 /* geo_object_id.cpp in Sources */, + 3901B745235F02B3006ABD43 /* cancellable.cpp in Sources */, 3D3731FE1F9A445500D2121B /* url_helpers.cpp in Sources */, 675341F91A3F57E400A0A8C3 /* src_point.cpp in Sources */, 675342031A3F57E400A0A8C3 /* strings_bundle.cpp in Sources */, @@ -796,7 +806,6 @@ 6753420A1A3F57E400A0A8C3 /* threaded_container.cpp in Sources */, 3917FA5C211E00BB00937DF4 /* pprof.cpp in Sources */, 675341FF1A3F57E400A0A8C3 /* string_format.cpp in Sources */, - 395784D12303034100F2CC07 /* url_helpers_tests.cpp in Sources */, 67A609AE1C88642E001E641A /* deferred_task.cpp in Sources */, 675341DF1A3F57E400A0A8C3 /* logging.cpp in Sources */, 671182F01C807C0A00CB8177 /* gmtime.cpp in Sources */, @@ -804,7 +813,6 @@ 675341E71A3F57E400A0A8C3 /* normalize_unicode.cpp in Sources */, 675341E11A3F57E400A0A8C3 /* lower_case.cpp in Sources */, 672DD4C11E0425600078E13C /* condition.cpp in Sources */, - 395784D02303034100F2CC07 /* thread_safe_queue_tests.cpp in Sources */, ); runOnlyForDeploymentPostprocessing = 0; };