From ade0ddcccb157a83f86adebfed54f6dd349d784c Mon Sep 17 00:00:00 2001
From: Tom0Brien <thomas16441@gmail.com>
Date: Wed, 21 Feb 2024 14:31:30 +1100
Subject: [PATCH] Add test that checks that pool idle triggers when a waiting
 task prevents running"

---
 src/threading/TaskScheduler.cpp | 12 +++++
 src/threading/TaskScheduler.hpp |  2 +
 tests/dsl/IdleSync.cpp          | 78 +++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 tests/dsl/IdleSync.cpp

diff --git a/src/threading/TaskScheduler.cpp b/src/threading/TaskScheduler.cpp
index f15585b64..6100d07aa 100644
--- a/src/threading/TaskScheduler.cpp
+++ b/src/threading/TaskScheduler.cpp
@@ -290,9 +290,21 @@ namespace threading {
                         // Erase the old position in the queue
                         queue.erase(it);
 
+                        if (pool->pool_descriptor.counts_for_idle && task.checked_runnable) {
+                            ++global_runnable_tasks;
+                            ++pool->runnable_tasks;
+                        }
+
                         // Return the task
                         return task;
                     }
+                    else if (!it->checked_runnable) {
+                        if (pool->pool_descriptor.counts_for_idle) {
+                            --global_runnable_tasks;
+                            --pool->runnable_tasks;
+                        }
+                        it->checked_runnable = true;
+                    }
                 }
 
                 // If pool concurrency is greater than group concurrency some threads can be left with nothing to do.
diff --git a/src/threading/TaskScheduler.hpp b/src/threading/TaskScheduler.hpp
index 814d6e2bd..937d855ad 100644
--- a/src/threading/TaskScheduler.hpp
+++ b/src/threading/TaskScheduler.hpp
@@ -103,6 +103,8 @@ namespace threading {
             util::ThreadPoolDescriptor thread_pool_descriptor;
             /// @brief The callback to be executed
             std::function<void()> run;
+            /// @brief If task has been checked for runnable
+            bool checked_runnable{false};
 
             /**
              * @brief Compare tasks based on their priority
diff --git a/tests/dsl/IdleSync.cpp b/tests/dsl/IdleSync.cpp
new file mode 100644
index 000000000..65c9f5c8f
--- /dev/null
+++ b/tests/dsl/IdleSync.cpp
@@ -0,0 +1,78 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2013 NUClear Contributors
+ *
+ * This file is part of the NUClear codebase.
+ * See https://github.com/Fastcode/NUClear for further info.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+ * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <catch.hpp>
+#include <nuclear>
+
+#include "test_util/TestBase.hpp"
+
+namespace {
+
+/// @brief A vector of events that have happened
+std::vector<std::string> events;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+
+class TestReactor : public test_util::TestBase<TestReactor, 1000> {
+public:
+    TestReactor(std::unique_ptr<NUClear::Environment> environment) : TestBase(std::move(environment), false) {
+
+        // Idle testing for default thread
+        on<Trigger<Step<1>>, Sync<TestReactor>>().then([this] {
+            events.push_back("Step 1 Start");
+            std::this_thread::sleep_for(std::chrono::milliseconds(100));
+            events.push_back("Step 1 End");
+        });
+        on<Trigger<Step<2>>, Sync<TestReactor>, MainThread>().then([this] { events.push_back("Step 2"); });
+        on<Idle<MainThread>>().then([this] {
+            events.push_back("Idle Main Thread");
+            powerplant.shutdown();
+        });
+
+        on<Startup>().then([this] {
+            emit(std::make_unique<Step<1>>());
+            emit(std::make_unique<Step<2>>());
+        });
+    }
+};
+
+}  // namespace
+
+TEST_CASE("Test that pool idle triggers when a waiting task prevents running", "[api][idle]") {
+
+    NUClear::Configuration config;
+    config.thread_count = 4;
+    NUClear::PowerPlant plant(config);
+    plant.install<TestReactor>();
+    plant.start();
+
+    const std::vector<std::string> expected = {
+        "Step 1 Start",
+        "Idle Main Thread",
+        "Step 1 End",
+        "Step 2",
+    };
+
+    // Make an info print the diff in an easy to read way if we fail
+    INFO(test_util::diff_string(expected, events));
+
+    // Check the events fired in order and only those events
+    REQUIRE(events == expected);
+}