SERVER-39936 Use PeriodicRunner handles to simplify shutdown ordering

7 files changed, 207 insertions, 232 deletions

diff --git a/src/mongo/util/SConscript b/src/mongo/util/SConscript
index 2c9dd3bfbc7..11e885ec0ae 100644
--- a/src/mongo/util/SConscript
+++ b/src/mongo/util/SConscript
@@ -162,6 +162,7 @@ env.Library(
         "periodic_runner.cpp",
     ],
     LIBDEPS=[
+        "$BUILD_DIR/mongo/base",
     ],
 )
 
diff --git a/src/mongo/util/mock_periodic_runner_impl.h b/src/mongo/util/mock_periodic_runner_impl.h
index 207fefe904d..361de33a4cd 100644
--- a/src/mongo/util/mock_periodic_runner_impl.h
+++ b/src/mongo/util/mock_periodic_runner_impl.h
@@ -40,25 +40,23 @@ namespace mongo {
  */
 class MockPeriodicRunnerImpl final : public PeriodicRunner {
 public:
-    class MockPeriodicJobHandleImpl final : public PeriodicRunner::PeriodicJobHandle {
+    class Job final : public ControllableJob {
     public:
-        ~MockPeriodicJobHandleImpl() = default;
-
-        void start() override{};
-        void stop() override{};
-        void pause() override{};
-        void resume() override{};
+        ~Job() = default;
+
+        void start() final{};
+        void stop() final{};
+        void pause() final{};
+        void resume() final{};
+        Milliseconds getPeriod() final{};
+        void setPeriod(Milliseconds ms) final{};
     };
 
     ~MockPeriodicRunnerImpl() = default;
 
-    std::unique_ptr<PeriodicRunner::PeriodicJobHandle> makeJob(PeriodicRunner::PeriodicJob job) {
-        return std::make_unique<MockPeriodicJobHandleImpl>();
+    PeriodicAnchor makeJob(PeriodicJob job) final {
+        return std::weak_ptr<Job>{};
     }
-
-    void scheduleJob(PeriodicRunner::PeriodicJob job) {}
-    void startup() {}
-    void shutdown() {}
 };
 
 }  // namespace mongo
diff --git a/src/mongo/util/periodic_runner.cpp b/src/mongo/util/periodic_runner.cpp
index dc54356142c..ed7069d4183 100644
--- a/src/mongo/util/periodic_runner.cpp
+++ b/src/mongo/util/periodic_runner.cpp
@@ -31,8 +31,58 @@
 
 #include "mongo/util/periodic_runner.h"
 
+#include "mongo/util/assert_util.h"
+
 namespace mongo {
 
 PeriodicRunner::~PeriodicRunner() = default;
 
+PeriodicJobAnchor::PeriodicJobAnchor(std::shared_ptr<Job> handle) : _handle{std::move(handle)} {}
+
+PeriodicJobAnchor::~PeriodicJobAnchor() {
+    if (!_handle) {
+        return;
+    }
+
+    _handle->stop();
+}
+
+void PeriodicJobAnchor::start() {
+    invariant(_handle);
+    _handle->start();
+}
+
+void PeriodicJobAnchor::pause() {
+    invariant(_handle);
+    _handle->pause();
+}
+
+void PeriodicJobAnchor::resume() {
+    invariant(_handle);
+    _handle->resume();
+}
+
+void PeriodicJobAnchor::stop() {
+    invariant(_handle);
+    _handle->stop();
+}
+
+void PeriodicJobAnchor::setPeriod(Milliseconds ms) {
+    invariant(_handle);
+    _handle->setPeriod(ms);
+}
+
+Milliseconds PeriodicJobAnchor::getPeriod() {
+    invariant(_handle);
+    return _handle->getPeriod();
+}
+
+void PeriodicJobAnchor::detach() {
+    _handle.reset();
+}
+
+bool PeriodicJobAnchor::isValid() const noexcept {
+    return static_cast<bool>(_handle);
+}
+
 }  // namespace mongo
diff --git a/src/mongo/util/periodic_runner.h b/src/mongo/util/periodic_runner.h
index 1508078a87f..93a03498357 100644
--- a/src/mongo/util/periodic_runner.h
+++ b/src/mongo/util/periodic_runner.h
@@ -30,13 +30,18 @@
 #pragma once
 
 #include <functional>
+#include <memory>
 #include <string>
 
+#include <boost/optional.hpp>
+
+#include "mongo/stdx/mutex.h"
 #include "mongo/util/time_support.h"
 
 namespace mongo {
 
 class Client;
+class PeriodicJobAnchor;
 
 /**
  * An interface for objects that run work items at specified intervals. Each individually scheduled
@@ -52,6 +57,7 @@ class Client;
 class PeriodicRunner {
 public:
     using Job = std::function<void(Client* client)>;
+    using JobAnchor = PeriodicJobAnchor;
 
     struct PeriodicJob {
         PeriodicJob(std::string name, Job callable, Milliseconds period)
@@ -73,9 +79,12 @@ public:
         Milliseconds interval;
     };
 
-    class PeriodicJobHandle {
+    /**
+     * A ControllableJob allows a user to reschedule the execution of a Job
+     */
+    class ControllableJob {
     public:
-        virtual ~PeriodicJobHandle() = default;
+        virtual ~ControllableJob() = default;
 
         /**
          * Starts running the job
@@ -115,39 +124,70 @@ public:
 
     virtual ~PeriodicRunner();
 
-
     /**
      * Creates a new job and adds it to the runner, but does not schedule it.
      * The caller is responsible for calling 'start' on the resulting handle in
      * order to begin the job running. This API should be used when the caller
      * is interested in observing and controlling the job execution state.
      */
-    virtual std::unique_ptr<PeriodicJobHandle> makeJob(PeriodicJob job) = 0;
+    virtual JobAnchor makeJob(PeriodicJob job) = 0;
+};
 
-    /**
-     * Schedules a job to be run at periodic intervals.
-     *
-     * If the runner is not running when a job is scheduled, that job should
-     * be saved so that it may run in the future once startup() is called.
-     */
-    virtual void scheduleJob(PeriodicJob job) = 0;
+/**
+ * A PeriodicJobAnchor allows the holder to control the scheduling of a job for the lifetime of the
+ * anchor. When an anchor is destructed, it stops its underlying job.
+ *
+ * The underlying weak_ptr for this class is not synchronized. In essence, treat use of this class
+ * as if it were a raw pointer to a ControllableJob.
+ *
+ * Each wrapped PeriodicRunner::ControllableJob function on this object throws
+ * if the underlying job is gone (e.g. in shutdown).
+ */
+class[[nodiscard]] PeriodicJobAnchor {
+public:
+    using Job = PeriodicRunner::ControllableJob;
+
+public:
+    // Note that this constructor is only intended for use with PeriodicRunner::makeJob()
+    explicit PeriodicJobAnchor(std::shared_ptr<Job> handle);
+
+    PeriodicJobAnchor() = default;
+    PeriodicJobAnchor(PeriodicJobAnchor &&) = default;
+    PeriodicJobAnchor& operator=(PeriodicJobAnchor&&) = default;
+
+    PeriodicJobAnchor(const PeriodicJobAnchor&) = delete;
+    PeriodicJobAnchor& operator=(const PeriodicJobAnchor&) = delete;
+
+    ~PeriodicJobAnchor();
+
+    void start();
+    void pause();
+    void resume();
+    void stop();
+    void setPeriod(Milliseconds ms);
+    Milliseconds getPeriod();
 
     /**
-     * Starts up this periodic runner.
+     * Abandon responsibility for scheduling the execution of this job
      *
-     * This method may safely be called multiple times, either with or without
-     * calls to shutdown() in between.
+     * This effectively invalidates the anchor.
      */
-    virtual void startup() = 0;
+    void detach();
 
     /**
-     * Shuts down this periodic runner. Stops all jobs from running.
+     * Returns if this PeriodicJobAnchor is associated with a PeriodicRunner::ControllableJob
      *
-     * This method may safely be called multiple times, either with or without
-     * calls to startup() in between. Any jobs that have been scheduled on this
-     * runner should no longer execute once shutdown() is called.
+     * This function is useful to see if a PeriodicJobAnchor is initialized. It does not necessarily
+     * inform whether a PeriodicJobAnchor will throw from a control function above.
      */
-    virtual void shutdown() = 0;
+    bool isValid() const noexcept;
+
+    explicit operator bool() const noexcept {
+        return isValid();
+    }
+
+private:
+    std::shared_ptr<Job> _handle;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/util/periodic_runner_impl.cpp b/src/mongo/util/periodic_runner_impl.cpp
index 8ffc8839bb8..fc21a7184a4 100644
--- a/src/mongo/util/periodic_runner_impl.cpp
+++ b/src/mongo/util/periodic_runner_impl.cpp
@@ -27,6 +27,8 @@
  *    it in the license file.
  */
 
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kDefault
+
 #include "mongo/platform/basic.h"
 
 #include "mongo/util/periodic_runner_impl.h"
@@ -34,6 +36,7 @@
 #include "mongo/db/client.h"
 #include "mongo/db/service_context.h"
 #include "mongo/util/clock_source.h"
+#include "mongo/util/log.h"
 #include "mongo/util/scopeguard.h"
 
 namespace mongo {
@@ -41,71 +44,41 @@ namespace mongo {
 PeriodicRunnerImpl::PeriodicRunnerImpl(ServiceContext* svc, ClockSource* clockSource)
     : _svc(svc), _clockSource(clockSource) {}
 
-PeriodicRunnerImpl::~PeriodicRunnerImpl() {
-    PeriodicRunnerImpl::shutdown();
-}
-
-std::shared_ptr<PeriodicRunnerImpl::PeriodicJobImpl> PeriodicRunnerImpl::createAndAddJob(
-    PeriodicJob job) {
+auto PeriodicRunnerImpl::makeJob(PeriodicJob job) -> JobAnchor {
     auto impl = std::make_shared<PeriodicJobImpl>(std::move(job), this->_clockSource, this->_svc);
 
-    stdx::lock_guard<stdx::mutex> lk(_mutex);
-    _jobs.push_back(impl);
-    return impl;
-}
-
-std::unique_ptr<PeriodicRunner::PeriodicJobHandle> PeriodicRunnerImpl::makeJob(PeriodicJob job) {
-    auto handle = std::make_unique<PeriodicJobHandleImpl>(createAndAddJob(job));
-    return std::move(handle);
+    JobAnchor anchor(std::move(impl));
+    return anchor;
 }
 
-void PeriodicRunnerImpl::scheduleJob(PeriodicJob job) {
-    auto impl = createAndAddJob(job);
-    stdx::lock_guard<stdx::mutex> lk(_mutex);
-    if (_running) {
-        impl->start();
-    }
-}
+PeriodicRunnerImpl::PeriodicJobImpl::PeriodicJobImpl(PeriodicJob job,
+                                                     ClockSource* source,
+                                                     ServiceContext* svc)
+    : _job(std::move(job)), _clockSource(source), _serviceContext(svc) {}
 
-void PeriodicRunnerImpl::startup() {
-    stdx::lock_guard<stdx::mutex> lk(_mutex);
+void PeriodicRunnerImpl::PeriodicJobImpl::_run() {
+    auto[startPromise, startFuture] = makePromiseFuture<void>();
 
-    if (_running) {
-        return;
+    {
+        stdx::lock_guard lk(_mutex);
+        invariant(_execStatus == ExecutionStatus::NOT_SCHEDULED);
     }
 
-    _running = true;
 
-    // schedule any jobs that we have
-    for (auto& job : _jobs) {
-        job->start();
-    }
-}
+    _thread = stdx::thread([ this, startPromise = std::move(startPromise) ]() mutable {
+        auto guard = makeGuard([this] { _stopPromise.emplaceValue(); });
 
-void PeriodicRunnerImpl::shutdown() {
-    stdx::lock_guard<stdx::mutex> lk(_mutex);
-    if (_running) {
-        _running = false;
+        Client::initThread(_job.name, _serviceContext, nullptr);
 
-        for (auto& job : _jobs) {
-            job->stop();
+        // Let start() know we're running
+        {
+            stdx::lock_guard lk(_mutex);
+            _execStatus = PeriodicJobImpl::ExecutionStatus::RUNNING;
         }
-        _jobs.clear();
-    }
-}
+        startPromise.emplaceValue();
 
-PeriodicRunnerImpl::PeriodicJobImpl::PeriodicJobImpl(PeriodicJob job,
-                                                     ClockSource* source,
-                                                     ServiceContext* svc)
-    : _job(std::move(job)), _clockSource(source), _serviceContext(svc) {}
-
-void PeriodicRunnerImpl::PeriodicJobImpl::_run() {
-    stdx::lock_guard<stdx::mutex> lk(_mutex);
-    invariant(_execStatus == PeriodicJobImpl::ExecutionStatus::NOT_SCHEDULED);
-    _thread = stdx::thread([this] {
-        Client::initThread(_job.name, _serviceContext, nullptr);
-        while (true) {
-            stdx::unique_lock<stdx::mutex> lk(_mutex);
+        stdx::unique_lock lk(_mutex);
+        while (_execStatus != ExecutionStatus::CANCELED) {
             // Wait until it's unpaused or canceled
             _condvar.wait(lk, [&] { return _execStatus != ExecutionStatus::PAUSED; });
             if (_execStatus == ExecutionStatus::CANCELED) {
@@ -135,10 +108,14 @@ void PeriodicRunnerImpl::PeriodicJobImpl::_run() {
             } while (_clockSource->now() < getDeadlineFromInterval());
         }
     });
-    _execStatus = PeriodicJobImpl::ExecutionStatus::RUNNING;
+
+    // Wait for the thread to actually start
+    startFuture.get();
 }
 
 void PeriodicRunnerImpl::PeriodicJobImpl::start() {
+    LOG(2) << "Starting periodic job " << _job.name;
+
     _run();
 }
 
@@ -158,17 +135,26 @@ void PeriodicRunnerImpl::PeriodicJobImpl::resume() {
 }
 
 void PeriodicRunnerImpl::PeriodicJobImpl::stop() {
-    {
+    auto lastExecStatus = [&] {
         stdx::lock_guard<stdx::mutex> lk(_mutex);
-        if (_execStatus != ExecutionStatus::RUNNING && _execStatus != ExecutionStatus::PAUSED)
-            return;
 
-        invariant(_thread.joinable());
+        return std::exchange(_execStatus, ExecutionStatus::CANCELED);
+    }();
 
-        _execStatus = PeriodicJobImpl::ExecutionStatus::CANCELED;
+    // If we never started, then nobody should wait
+    if (lastExecStatus == ExecutionStatus::NOT_SCHEDULED) {
+        return;
     }
-    _condvar.notify_one();
-    _thread.join();
+
+    // Only join once
+    if (lastExecStatus != ExecutionStatus::CANCELED) {
+        LOG(2) << "Stopping periodic job " << _job.name;
+
+        _condvar.notify_one();
+        _thread.join();
+    }
+
+    _stopPromise.getFuture().get();
 }
 
 Milliseconds PeriodicRunnerImpl::PeriodicJobImpl::getPeriod() {
@@ -185,50 +171,4 @@ void PeriodicRunnerImpl::PeriodicJobImpl::setPeriod(Milliseconds ms) {
     }
 }
 
-namespace {
-
-template <typename T>
-std::shared_ptr<T> lockAndAssertExists(std::weak_ptr<T> ptr, StringData errMsg) {
-    if (auto p = ptr.lock()) {
-        return p;
-    } else {
-        uasserted(ErrorCodes::InternalError, errMsg);
-    }
-}
-
-constexpr auto kPeriodicJobHandleLifetimeErrMsg =
-    "The PeriodicRunner job for this handle no longer exists"_sd;
-
-}  // namespace
-
-void PeriodicRunnerImpl::PeriodicJobHandleImpl::start() {
-    auto job = lockAndAssertExists(_jobWeak, kPeriodicJobHandleLifetimeErrMsg);
-    job->start();
-}
-
-void PeriodicRunnerImpl::PeriodicJobHandleImpl::stop() {
-    auto job = lockAndAssertExists(_jobWeak, kPeriodicJobHandleLifetimeErrMsg);
-    job->stop();
-}
-
-void PeriodicRunnerImpl::PeriodicJobHandleImpl::pause() {
-    auto job = lockAndAssertExists(_jobWeak, kPeriodicJobHandleLifetimeErrMsg);
-    job->pause();
-}
-
-void PeriodicRunnerImpl::PeriodicJobHandleImpl::resume() {
-    auto job = lockAndAssertExists(_jobWeak, kPeriodicJobHandleLifetimeErrMsg);
-    job->resume();
-}
-
-Milliseconds PeriodicRunnerImpl::PeriodicJobHandleImpl::getPeriod() {
-    auto job = lockAndAssertExists(_jobWeak, kPeriodicJobHandleLifetimeErrMsg);
-    return job->getPeriod();
-}
-
-void PeriodicRunnerImpl::PeriodicJobHandleImpl::setPeriod(Milliseconds ms) {
-    auto job = lockAndAssertExists(_jobWeak, kPeriodicJobHandleLifetimeErrMsg);
-    job->setPeriod(ms);
-}
-
 }  // namespace mongo
diff --git a/src/mongo/util/periodic_runner_impl.h b/src/mongo/util/periodic_runner_impl.h
index eb6663d708b..a921a66c59f 100644
--- a/src/mongo/util/periodic_runner_impl.h
+++ b/src/mongo/util/periodic_runner_impl.h
@@ -36,6 +36,7 @@
 #include "mongo/stdx/mutex.h"
 #include "mongo/stdx/thread.h"
 #include "mongo/util/clock_source.h"
+#include "mongo/util/future.h"
 #include "mongo/util/periodic_runner.h"
 
 namespace mongo {
@@ -50,17 +51,11 @@ class ServiceContext;
 class PeriodicRunnerImpl : public PeriodicRunner {
 public:
     PeriodicRunnerImpl(ServiceContext* svc, ClockSource* clockSource);
-    ~PeriodicRunnerImpl();
 
-    std::unique_ptr<PeriodicRunner::PeriodicJobHandle> makeJob(PeriodicJob job) override;
-    void scheduleJob(PeriodicJob job) override;
-
-    void startup() override;
-
-    void shutdown() override;
+    JobAnchor makeJob(PeriodicJob job) override;
 
 private:
-    class PeriodicJobImpl {
+    class PeriodicJobImpl : public ControllableJob {
         PeriodicJobImpl(const PeriodicJobImpl&) = delete;
         PeriodicJobImpl& operator=(const PeriodicJobImpl&) = delete;
 
@@ -68,12 +63,12 @@ private:
         friend class PeriodicRunnerImpl;
         PeriodicJobImpl(PeriodicJob job, ClockSource* source, ServiceContext* svc);
 
-        void start();
-        void pause();
-        void resume();
-        void stop();
-        Milliseconds getPeriod();
-        void setPeriod(Milliseconds ms);
+        void start() override;
+        void pause() override;
+        void resume() override;
+        void stop() override;
+        Milliseconds getPeriod() override;
+        void setPeriod(Milliseconds ms) override;
 
         enum class ExecutionStatus { NOT_SCHEDULED, RUNNING, PAUSED, CANCELED };
 
@@ -83,7 +78,9 @@ private:
         PeriodicJob _job;
         ClockSource* _clockSource;
         ServiceContext* _serviceContext;
+
         stdx::thread _thread;
+        SharedPromise<void> _stopPromise;
 
         stdx::mutex _mutex;
         stdx::condition_variable _condvar;
@@ -95,28 +92,8 @@ private:
 
     std::shared_ptr<PeriodicRunnerImpl::PeriodicJobImpl> createAndAddJob(PeriodicJob job);
 
-    class PeriodicJobHandleImpl : public PeriodicJobHandle {
-    public:
-        explicit PeriodicJobHandleImpl(std::weak_ptr<PeriodicJobImpl> jobImpl)
-            : _jobWeak(jobImpl) {}
-        void start() override;
-        void stop() override;
-        void pause() override;
-        void resume() override;
-        Milliseconds getPeriod() override;
-        void setPeriod(Milliseconds ms) override;
-
-    private:
-        std::weak_ptr<PeriodicJobImpl> _jobWeak;
-    };
-
     ServiceContext* _svc;
     ClockSource* _clockSource;
-
-    std::vector<std::shared_ptr<PeriodicJobImpl>> _jobs;
-
-    stdx::mutex _mutex;
-    bool _running = false;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/util/periodic_runner_impl_test.cpp b/src/mongo/util/periodic_runner_impl_test.cpp
index fde21f4dd86..86f2f1a96d6 100644
--- a/src/mongo/util/periodic_runner_impl_test.cpp
+++ b/src/mongo/util/periodic_runner_impl_test.cpp
@@ -51,10 +51,6 @@ public:
         _runner = std::make_unique<PeriodicRunnerImpl>(getServiceContext(), _clockSource.get());
     }
 
-    void tearDown() override {
-        _runner->shutdown();
-    }
-
     ClockSourceMock& clockSource() {
         return *_clockSource;
     }
@@ -72,7 +68,6 @@ class PeriodicRunnerImplTest : public PeriodicRunnerImplTestNoSetup {
 public:
     void setUp() override {
         PeriodicRunnerImplTestNoSetup::setUp();
-        runner().startup();
     }
 };
 
@@ -94,7 +89,8 @@ TEST_F(PeriodicRunnerImplTest, OneJobTest) {
                                     },
                                     interval);
 
-    runner().scheduleJob(std::move(job));
+    auto jobAnchor = runner().makeJob(std::move(job));
+    jobAnchor.start();
 
     // Fast forward ten times, we should run all ten times.
     for (int i = 0; i < 10; i++) {
@@ -126,7 +122,7 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobDoesNotRunWithoutStart) {
                                     },
                                     interval);
 
-    auto handle = runner().makeJob(std::move(job));
+    auto jobAnchor = runner().makeJob(std::move(job));
     clockSource().advance(interval);
     ASSERT_EQ(count, 0);
 
@@ -151,8 +147,8 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobRunsCorrectlyWithStart) {
                                     },
                                     interval);
 
-    auto handle = runner().makeJob(std::move(job));
-    handle->start();
+    auto jobAnchor = runner().makeJob(std::move(job));
+    jobAnchor.start();
     // Fast forward ten times, we should run all ten times.
     for (int i = 0; i < 10; i++) {
         {
@@ -186,8 +182,8 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobPausesCorrectly) {
                                     },
                                     interval);
 
-    auto handle = runner().makeJob(std::move(job));
-    handle->start();
+    auto jobAnchor = runner().makeJob(std::move(job));
+    jobAnchor.start();
     // Wait for the first execution.
     {
         stdx::unique_lock<stdx::mutex> lk(mutex);
@@ -197,7 +193,7 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobPausesCorrectly) {
     {
         stdx::unique_lock<stdx::mutex> lk(mutex);
         isPaused = true;
-        handle->pause();
+        jobAnchor.pause();
     }
 
     // Fast forward ten times, we shouldn't run anymore. If we do, the assert inside the job will
@@ -228,8 +224,8 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobResumesCorrectly) {
                                     },
                                     interval);
 
-    auto handle = runner().makeJob(std::move(job));
-    handle->start();
+    auto jobAnchor = runner().makeJob(std::move(job));
+    jobAnchor.start();
     // Wait for the first execution.
     {
         stdx::unique_lock<stdx::mutex> lk(mutex);
@@ -252,7 +248,7 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobResumesCorrectly) {
         }
     }
 
-    handle->pause();
+    jobAnchor.pause();
 
     // Fast forward ten times, we shouldn't run anymore.
     for (int i = 0; i < 10; i++) {
@@ -262,7 +258,7 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobResumesCorrectly) {
     // Make sure we didn't run anymore while paused.
     ASSERT_EQ(count, numIterationsBeforePause);
 
-    handle->resume();
+    jobAnchor.resume();
     // Fast forward, we should run at least once.
     clockSource().advance(interval);
 
@@ -275,39 +271,6 @@ TEST_F(PeriodicRunnerImplTest, OnePausableJobResumesCorrectly) {
     tearDown();
 }
 
-TEST_F(PeriodicRunnerImplTestNoSetup, ScheduleBeforeStartupTest) {
-    int count = 0;
-    Milliseconds interval{5};
-
-    stdx::mutex mutex;
-    stdx::condition_variable cv;
-
-    // Schedule a job before startup
-    PeriodicRunner::PeriodicJob job("job",
-                                    [&count, &mutex, &cv](Client*) {
-                                        {
-                                            stdx::unique_lock<stdx::mutex> lk(mutex);
-                                            count++;
-                                        }
-                                        cv.notify_all();
-                                    },
-                                    interval);
-
-    runner().scheduleJob(std::move(job));
-
-    // Start the runner, job should still run
-    runner().startup();
-
-    clockSource().advance(interval);
-
-    {
-        stdx::unique_lock<stdx::mutex> lk(mutex);
-        cv.wait(lk, [&count] { return count > 0; });
-    }
-
-    tearDown();
-}
-
 TEST_F(PeriodicRunnerImplTest, TwoJobsTest) {
     int countA = 0;
     int countB = 0;
@@ -338,8 +301,11 @@ TEST_F(PeriodicRunnerImplTest, TwoJobsTest) {
                                      },
                                      intervalB);
 
-    runner().scheduleJob(std::move(jobA));
-    runner().scheduleJob(std::move(jobB));
+    auto jobAnchorA = runner().makeJob(std::move(jobA));
+    auto jobAnchorB = runner().makeJob(std::move(jobB));
+
+    jobAnchorA.start();
+    jobAnchorB.start();
 
     // Fast forward and wait for both jobs to run the right number of times
     for (int i = 0; i <= 10; i++) {
@@ -382,8 +348,11 @@ TEST_F(PeriodicRunnerImplTest, TwoJobsDontDeadlock) {
                                      },
                                      Milliseconds(1));
 
-    runner().scheduleJob(std::move(jobA));
-    runner().scheduleJob(std::move(jobB));
+    auto jobAnchorA = runner().makeJob(std::move(jobA));
+    auto jobAnchorB = runner().makeJob(std::move(jobB));
+
+    jobAnchorA.start();
+    jobAnchorB.start();
 
     clockSource().advance(Milliseconds(1));
 
@@ -415,16 +384,16 @@ TEST_F(PeriodicRunnerImplTest, ChangingIntervalWorks) {
                                     },
                                     Milliseconds(5));
 
-    auto handle = runner().makeJob(std::move(job));
-    handle->start();
+    auto jobAnchor = runner().makeJob(std::move(job));
+    jobAnchor.start();
     // Wait for the first execution.
     {
         stdx::unique_lock<stdx::mutex> lk(mutex);
         cv.wait(lk, [&] { return timesCalled; });
     }
 
-    handle->setPeriod(Milliseconds(10));
-    ASSERT_EQ(handle->getPeriod(), Milliseconds(10));
+    jobAnchor.setPeriod(Milliseconds(10));
+    ASSERT_EQ(jobAnchor.getPeriod(), Milliseconds(10));
 
     // if we change the period to a longer duration, that doesn't trigger a run
     {
@@ -456,8 +425,8 @@ TEST_F(PeriodicRunnerImplTest, ChangingIntervalWorks) {
         ASSERT_EQ(timesCalled, 2ul);
     }
 
-    handle->setPeriod(Milliseconds(4));
-    ASSERT_EQ(handle->getPeriod(), Milliseconds(4));
+    jobAnchor.setPeriod(Milliseconds(4));
+    ASSERT_EQ(jobAnchor.getPeriod(), Milliseconds(4));
 
     // shortening triggers the period
     {