This is an automated email from the ASF dual-hosted git repository.
hsluoyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/casbin-cpp.git
The following commit(s) were added to refs/heads/master by this push:
new adec06b fix: bound Ticker callbacks and make policy loading stop
safely (#282)
adec06b is described below
commit adec06bcb0aadd9392ec9f734fcf25c4b4274a89
Author: Emery <[email protected]>
AuthorDate: Sat Nov 7 10:13:34 2026 +0800
fix: bound Ticker callbacks and make policy loading stop safely (#282)
---
casbin/enforcer_synced.cpp | 5 ++-
casbin/util/ticker.cpp | 32 ++++++++++------
include/casbin/util/ticker.h | 9 ++++-
tests/CMakeLists.txt | 1 +
tests/ticker_test.cpp | 88 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 119 insertions(+), 16 deletions(-)
diff --git a/casbin/enforcer_synced.cpp b/casbin/enforcer_synced.cpp
index cf8c1c7..1c167f9 100644
--- a/casbin/enforcer_synced.cpp
+++ b/casbin/enforcer_synced.cpp
@@ -122,7 +122,8 @@ bool SyncedEnforcer ::IsAutoLoadingRunning() {
// StopAutoLoadPolicy causes the thread to exit
void SyncedEnforcer ::StopAutoLoadPolicy() {
- ticker->stop();
+ if (ticker)
+ ticker->stop();
autoLoadRunning = false;
}
@@ -590,4 +591,4 @@ bool SyncedEnforcer::SyncedEnforceWithMatcher(const
std::string& matcher, const
} // namespace casbin
-#endif // ENFORCER_SYNCED_CPP
\ No newline at end of file
+#endif // ENFORCER_SYNCED_CPP
diff --git a/casbin/util/ticker.cpp b/casbin/util/ticker.cpp
index 90dfeca..45009fc 100644
--- a/casbin/util/ticker.cpp
+++ b/casbin/util/ticker.cpp
@@ -33,29 +33,37 @@ Ticker::~Ticker() {
}
void Ticker::start() {
+ std::lock_guard<std::mutex> lifecycle(_lifecycleMutex);
if (_running) return;
+ if (_worker.joinable())
+ _worker.join();
_running = true;
- _futures1.push_back(std::async(std::launch::async, &Ticker::timer_loop,
this));
+ _worker = std::thread(&Ticker::timer_loop, this);
}
void Ticker::stop() {
- _running = false;
- std::lock_guard<std::mutex> lock(_tickIntervalMutex);
- for (auto&& f : _futures1) {
- f.wait();
- }
- for (auto&& f : _futures2) {
- f.wait();
+ std::lock_guard<std::mutex> lifecycle(_lifecycleMutex);
+ {
+ std::lock_guard<std::mutex> lock(_tickIntervalMutex);
+ _running = false;
}
+ _wake.notify_all();
+ if (_worker.joinable() && _worker.get_id() != std::this_thread::get_id())
+ _worker.join();
}
void Ticker::timer_loop() {
+ std::unique_lock<std::mutex> lock(_tickIntervalMutex);
while (_running) {
- {
- std::lock_guard<std::mutex> lock(_tickIntervalMutex);
- _futures2.push_back(std::async(std::launch::async, _onTick));
- std::this_thread::sleep_for(_tickInterval);
+ lock.unlock();
+ try {
+ _onTick();
+ } catch (...) {
+ // The previous async callbacks also kept exceptions in their
+ // futures. Keep a callback failure from terminating the scheduler.
}
+ lock.lock();
+ _wake.wait_for(lock, _tickInterval, [this] { return !_running.load();
});
}
}
diff --git a/include/casbin/util/ticker.h b/include/casbin/util/ticker.h
index 1d6a12c..f3ed426 100644
--- a/include/casbin/util/ticker.h
+++ b/include/casbin/util/ticker.h
@@ -18,9 +18,13 @@
#define TICKER_H
#include <chrono>
+#include <atomic>
+#include <condition_variable>
#include <functional>
#include <future>
#include <mutex>
+#include <thread>
+#include <vector>
namespace casbin {
@@ -43,9 +47,10 @@ private:
on_tick_t _onTick;
tick_interval_t _tickInterval;
std::atomic_bool _running;
+ std::mutex _lifecycleMutex;
std::mutex _tickIntervalMutex;
- future_vec _futures1;
- future_vec _futures2;
+ std::condition_variable _wake;
+ std::thread _worker;
};
}; // namespace casbin
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 1b94ae1..3fd5b15 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -24,6 +24,7 @@ if(CASBIN_BUILD_TEST)
set(CASBIN_TEST_SOURCE
built_in_functions_test.cpp
+ ticker_test.cpp
config_test.cpp
enforcer_test.cpp
enforcer_cached_test.cpp
diff --git a/tests/ticker_test.cpp b/tests/ticker_test.cpp
new file mode 100644
index 0000000..d2fbb8f
--- /dev/null
+++ b/tests/ticker_test.cpp
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <gtest/gtest.h>
+#include <atomic>
+#include <chrono>
+#include <condition_variable>
+#include <future>
+#include <mutex>
+#include <thread>
+
+#include "casbin/enforcer_synced.h"
+#include "casbin/util/ticker.h"
+
+namespace {
+
+TEST(ReleaseTicker, SlowCallbacksDoNotOverlapAndCanRestart) {
+ std::atomic<int> active{0};
+ std::atomic<int> calls{0};
+ std::atomic<bool> overlap{false};
+ std::mutex progressMutex;
+ std::condition_variable progress;
+ casbin::Ticker ticker([&] {
+ if (active.fetch_add(1) != 0)
+ overlap = true;
+ {
+ std::lock_guard<std::mutex> lock(progressMutex);
+ ++calls;
+ }
+ progress.notify_one();
+ std::this_thread::sleep_for(std::chrono::milliseconds(20));
+ --active;
+ }, std::chrono::milliseconds(1));
+ const auto waitForCalls = [&](int minimum) {
+ std::unique_lock<std::mutex> lock(progressMutex);
+ return progress.wait_for(lock, std::chrono::seconds(5), [&] { return
calls.load() >= minimum; });
+ };
+ ticker.start();
+ // Wait for observable callback progress, including on busy CI runners.
+ EXPECT_TRUE(waitForCalls(3));
+ ticker.stop();
+ EXPECT_FALSE(overlap);
+ EXPECT_EQ(active.load(), 0);
+ EXPECT_GE(calls.load(), 1);
+ const auto before = calls.load();
+ ticker.start();
+ EXPECT_TRUE(waitForCalls(before + 1));
+ ticker.stop();
+ ticker.stop();
+ EXPECT_GT(calls.load(), before);
+ EXPECT_FALSE(overlap);
+}
+
+TEST(ReleaseTicker, StopInterruptsLongWait) {
+ std::promise<void> entered;
+ casbin::Ticker ticker([&] { entered.set_value(); },
std::chrono::seconds(10));
+ ticker.start();
+ auto ready = entered.get_future().wait_for(std::chrono::seconds(2));
+ EXPECT_EQ(ready, std::future_status::ready);
+ const auto before = std::chrono::steady_clock::now();
+ ticker.stop();
+ EXPECT_LT(std::chrono::steady_clock::now() - before,
std::chrono::seconds(2));
+}
+
+TEST(ReleaseTicker, AutoLoadStopIsSafeBeforeStart) {
+ casbin::SyncedEnforcer enforcer;
+ enforcer.StopAutoLoadPolicy();
+ enforcer.StopAutoLoadPolicy();
+ EXPECT_FALSE(enforcer.IsAutoLoadingRunning());
+}
+
+} // namespace
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
