ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-24 15:40:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Stop embedded tests from hanging on ARM by using atomic_flag (#6248)

Browse Source 
This change replaces the mutex `stoppingMutex_`, the `atomic_bool` variable `isTimeToStop`, and the conditional variable `stoppingCondition_` with an `atomic_flag` variable.

When `xrpld` is running the embedded tests as a child process, it has a control thread (the app bundle thread) that starts the application, and an application thread (the thread that executes `app_->run()`). Due to the relaxed memory ordering on ARM, it's not guaranteed that the application thread can see the change of the value resulting from the `isTimeToStop.exchange(true)` call before it is notified by `stoppingCondition_.notify_all()`, even though they do happen in the right order in the app bundle thread in `ApplicationImp::signalStop`. We therefore often get into the situation where `isTimeToStop` is `true`, but the application thread is waiting for `stoppingCondition_` to notify, because the app bundle thread may have already notified before the application thread actually starts waiting.

Switching to a single `atomic_flag` variable makes sure that there's only one synchronisation object and then the memory order guarantee provided by c++ can make sure that `notify_all` gets synchronised after `test_and_set` does.

Fixing this issue will stop the unit tests hanging forever and then we should see less (or hopefully no) time out errors in daily github action runs
This commit is contained in:
Jingchen
2026-01-26 21:39:28 +00:00
committed by GitHub
parent a2f1973574
commit bb529d0317
1 changed files with 5 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
src/xrpld/app/main/Application.cpp
View File

@@ -203,11 +203,7 @@ public:
boost::asio::signal_set m_signals;
// Once we get C++20, we could use `std::atomic_flag` for `isTimeToStop`
// and eliminate the need for the condition variable and the mutex.
std::condition_variable stoppingCondition_;
mutable std::mutex stoppingMutex_;
std::atomic<bool> isTimeToStop = false;
std::atomic_flag isTimeToStop;
std::atomic<bool> checkSigs_;
@@ -1539,10 +1535,7 @@ ApplicationImp::run()
getLoadManager().activateStallDetector();
}
{
std::unique_lock<std::mutex> lk{stoppingMutex_};
stoppingCondition_.wait(lk, [this] { return isTimeToStop.load(); });
}
isTimeToStop.wait(false, std::memory_order_relaxed);
JLOG(m_journal.debug()) << "Application stopping";
@@ -1629,14 +1622,14 @@ ApplicationImp::run()
void
ApplicationImp::signalStop(std::string msg)
{
if (!isTimeToStop.exchange(true))
if (!isTimeToStop.test_and_set(std::memory_order_acquire))
{
if (msg.empty())
JLOG(m_journal.warn()) << "Server stopping";
else
JLOG(m_journal.warn()) << "Server stopping: " << msg;
stoppingCondition_.notify_all();
isTimeToStop.notify_all();
}
}
@@ -1655,7 +1648,7 @@ ApplicationImp::checkSigs(bool check)
bool
ApplicationImp::isStopping() const
{
return isTimeToStop.load();
return isTimeToStop.test(std::memory_order_relaxed);
}
int