diff --git a/centipede/periodic_action.cc b/centipede/periodic_action.cc index f4192bce..2336d958 100644 --- a/centipede/periodic_action.cc +++ b/centipede/periodic_action.cc @@ -14,8 +14,11 @@ #include "./centipede/periodic_action.h" +#include +#include #include +#include "absl/base/thread_annotations.h" #include "absl/functional/any_invocable.h" #include "absl/log/check.h" #include "absl/synchronization/mutex.h" @@ -24,65 +27,93 @@ namespace centipede { -PeriodicAction::PeriodicAction( // - absl::AnyInvocable action, Options options) - : action_{std::move(action)}, - options_{std::move(options)}, - thread_{[this]() { RunLoop(); }} { - // NOTE: Allow `options_.delay` to be `absl::InfiniteDuration()`: that's a - // valid use case, where the run-loop actually starts looping only after a - // first explicit nudge. - CHECK_GT(options_.interval, absl::ZeroDuration()); -} - -PeriodicAction::~PeriodicAction() { Stop(); } - -void PeriodicAction::Stop() { - StopAsync(); - // The run-loop should exit the next time it checks `stop_`. Note that if - // the loop is currently in the middle of an invocation of `action_`, it - // will wait for the invocation to finish, so we might block here for an - // `action_`-dependent amount of time. - if (thread_.joinable()) { - thread_.join(); +class PeriodicAction::Impl { + public: + Impl(absl::AnyInvocable action, PeriodicAction::Options options) + : action_{std::move(action)}, + options_{std::move(options)}, + thread_{[this]() { RunLoop(); }} { + // NOTE: Allow `options_.delay` to be `absl::InfiniteDuration()`: that's a + // valid use case, where the run-loop actually starts looping only after a + // first explicit nudge. + CHECK_GT(options_.interval, absl::ZeroDuration()); } -} - -void PeriodicAction::StopAsync() { - if (!stop_.HasBeenNotified()) { - // Prime the run-loop to exit next time it re-checks `stop_`. - stop_.Notify(); - // Nudge the run-loop out of the sleeping phase, if it's there: the loop - // immediately goes to re-check `stop_` and exits. - { - absl::MutexLock lock{&nudge_mu_}; - nudge_ = true; + + void Stop() { + StopAsync(); + // The run-loop should exit the next time it checks `stop_`. Note that if + // the loop is currently in the middle of an invocation of `action_`, it + // will wait for the invocation to finish, so we might block here for an + // `action_`-dependent amount of time. + if (thread_.joinable()) { + thread_.join(); } } -} - -void PeriodicAction::Nudge() { - absl::MutexLock lock{&nudge_mu_}; - nudge_ = true; -} - -void PeriodicAction::RunLoop() { - SleepUnlessWokenByNudge(options_.delay); - while (!stop_.HasBeenNotified()) { - action_(); - SleepUnlessWokenByNudge(options_.interval); + + void StopAsync() { + if (!stop_.HasBeenNotified()) { + // Prime the run-loop to exit next time it re-checks `stop_`. + stop_.Notify(); + // Nudge the run-loop out of the sleeping phase, if it's there: the loop + // immediately goes to re-check `stop_` and exits. + { + absl::MutexLock lock{&nudge_mu_}; + nudge_ = true; + } + } } -} - -void PeriodicAction::SleepUnlessWokenByNudge(absl::Duration duration) { - if (nudge_mu_.WriterLockWhenWithTimeout(absl::Condition{&nudge_}, duration)) { - // Got woken up by a nudge. - nudge_mu_.AssertHeld(); - nudge_ = false; - } else { - // A nudge never came, slept well the entire time: nothing to do. + + void Nudge() { + absl::MutexLock lock{&nudge_mu_}; + nudge_ = true; + } + + private: + void RunLoop() { + SleepUnlessWokenByNudge(options_.delay); + while (!stop_.HasBeenNotified()) { + action_(); + SleepUnlessWokenByNudge(options_.interval); + } + } + + void SleepUnlessWokenByNudge(absl::Duration duration) { + if (nudge_mu_.WriterLockWhenWithTimeout( // + absl::Condition{&nudge_}, duration)) { + // Got woken up by a nudge. + nudge_mu_.AssertHeld(); + nudge_ = false; + } else { + // A nudge never came, slept well the entire time: nothing to do. + } + nudge_mu_.Unlock(); } - nudge_mu_.Unlock(); -} + + absl::AnyInvocable action_; + PeriodicAction::Options options_; + + // WARNING!!! The order below is important. + absl::Notification stop_; + absl::Mutex nudge_mu_; + bool nudge_ ABSL_GUARDED_BY(nudge_mu_) = false; + std::thread thread_; +}; + +PeriodicAction::PeriodicAction( // + absl::AnyInvocable action, Options options) + : pimpl_{std::make_unique(std::move(action), std::move(options))} {} + +PeriodicAction::~PeriodicAction() { pimpl_->Stop(); } + +void PeriodicAction::Stop() { pimpl_->Stop(); } + +void PeriodicAction::StopAsync() { pimpl_->StopAsync(); } + +void PeriodicAction::Nudge() { pimpl_->Nudge(); } + +// NOTE: Even though these are defaulted, they still must be defined here in the +// .cc, because `Impl` is an incomplete type in the .h. +PeriodicAction::PeriodicAction(PeriodicAction&&) = default; +PeriodicAction& PeriodicAction::operator=(PeriodicAction&&) = default; } // namespace centipede diff --git a/centipede/periodic_action.h b/centipede/periodic_action.h index 4cc4f254..7ed4fbbc 100644 --- a/centipede/periodic_action.h +++ b/centipede/periodic_action.h @@ -33,12 +33,9 @@ #ifndef FUZZTEST_CENTIPEDE_PERIODIC_ACTION_H_ #define FUZZTEST_CENTIPEDE_PERIODIC_ACTION_H_ -#include +#include -#include "absl/base/thread_annotations.h" #include "absl/functional/any_invocable.h" -#include "absl/synchronization/mutex.h" -#include "absl/synchronization/notification.h" #include "absl/time/time.h" namespace centipede { @@ -58,11 +55,9 @@ class PeriodicAction { PeriodicAction(absl::AnyInvocable action, Options options); - // Non-copyable and non-movable. - PeriodicAction(const PeriodicAction&) = delete; - PeriodicAction& operator=(const PeriodicAction&) = delete; - PeriodicAction(PeriodicAction&&) = delete; - PeriodicAction& operator=(PeriodicAction&&) = delete; + // Movable, but not copyable. + PeriodicAction(PeriodicAction&&); + PeriodicAction& operator=(PeriodicAction&&); // Stops the periodic action via RAII. May block: waits for any currently // active invocation of the action to finish first before returning. @@ -91,15 +86,10 @@ class PeriodicAction { // which case wakes up and returns immediately. void SleepUnlessWokenByNudge(absl::Duration duration); - absl::AnyInvocable action_; - const Options options_; - - // WARNING!!! The order below is important. - - absl::Notification stop_; - absl::Mutex nudge_mu_; - bool nudge_ ABSL_GUARDED_BY(nudge_mu_) = false; - std::thread thread_; + // Use the "pointer to implementation" idiom to make the class movable and + // move-constructible. + class Impl; + std::unique_ptr pimpl_; }; } // namespace centipede diff --git a/centipede/rusage_profiler.cc b/centipede/rusage_profiler.cc index fdb044f2..298e4250 100644 --- a/centipede/rusage_profiler.cc +++ b/centipede/rusage_profiler.cc @@ -425,16 +425,15 @@ void RUsageProfiler::StartTimelapse( // std::string title) { absl::WriterMutexLock lock{&mutex_}; CHECK(!timelapse_recorder_) << "StopTimelapse() wasn't called"; - const PeriodicAction::Options periodic_options{ - .delay = absl::ZeroDuration(), - .interval = interval, - }; timelapse_recorder_ = std::make_unique( [this, loc = std::move(loc), title = std::move(title), also_log]() { const auto& s = TakeSnapshot(loc, title); if (also_log) s.Log(); }, - periodic_options); + PeriodicAction::Options{ + .delay = absl::ZeroDuration(), + .interval = interval, + }); } void RUsageProfiler::StopTimelapse() {