Skip to content

Commit 0d605d2

Browse files
authored
Fix realtime publisher race condition upon initialization (#309)
1 parent f45ba91 commit 0d605d2

File tree

1 file changed

+15
-8
lines changed

1 file changed

+15
-8
lines changed

realtime_tools/include/realtime_tools/realtime_publisher.hpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ class RealtimePublisher
7878
: publisher_(publisher), is_running_(false), keep_running_(true), turn_(State::LOOP_NOT_STARTED)
7979
{
8080
thread_ = std::thread(&RealtimePublisher::publishingLoop, this);
81+
82+
// Wait for the thread to be ready before proceeding
83+
// This is important to ensure that the thread is properly initialized and ready to handle
84+
// messages before any other operations are performed on the RealtimePublisher instance.
85+
while (!thread_.joinable() ||
86+
turn_.load(std::memory_order_acquire) == State::LOOP_NOT_STARTED) {
87+
std::this_thread::sleep_for(std::chrono::microseconds(100));
88+
}
8189
}
8290

8391
[[deprecated(
@@ -116,14 +124,14 @@ class RealtimePublisher
116124
/**
117125
* \brief Try to acquire the data lock for non-realtime message publishing
118126
*
119-
* It first checks if the current state allows non-realtime message publishing (turn_ == NON_REALTIME)
127+
* It first checks if the current state allows non-realtime message publishing (turn_ == REALTIME)
120128
* and then attempts to lock
121129
*
122130
* \return true if the lock was successfully acquired, false otherwise
123131
*/
124132
bool trylock()
125133
{
126-
if (turn_.load(std::memory_order_acquire) == State::NON_REALTIME && msg_mutex_.try_lock()) {
134+
if (turn_.load(std::memory_order_acquire) == State::REALTIME && msg_mutex_.try_lock()) {
127135
return true;
128136
} else {
129137
return false;
@@ -159,7 +167,7 @@ class RealtimePublisher
159167
*/
160168
void unlockAndPublish()
161169
{
162-
turn_.store(State::REALTIME, std::memory_order_release);
170+
turn_.store(State::NON_REALTIME, std::memory_order_release);
163171
unlock();
164172
}
165173

@@ -196,33 +204,32 @@ class RealtimePublisher
196204
* \brief Publishing loop (runs in separate thread)
197205
*
198206
* This is the main loop for the non-realtime publishing thread. It:
199-
* 1. Waits for new messages (State::REALTIME)
207+
* 1. Waits for new messages (State::NON_REALTIME)
200208
* 2. Copies the message data
201209
* 3. Publishes the message through the ROS publisher
202-
* 4. Returns to State::NON_REALTIME to allow realtime updates
210+
* 4. Returns to State::REALTIME to allow realtime updates
203211
*
204212
* The loop continues until keep_running_ is set to false.
205213
*/
206214
void publishingLoop()
207215
{
208216
is_running_ = true;
209-
turn_.store(State::NON_REALTIME, std::memory_order_release);
210217

211218
while (keep_running_) {
212219
MessageT outgoing;
213220

214221
{
222+
turn_.store(State::REALTIME, std::memory_order_release);
215223
// Locks msg_ and copies it to outgoing
216224
std::unique_lock<std::mutex> lock_(msg_mutex_);
217-
updated_cond_.wait(lock_, [&] { return turn_ == State::REALTIME || !keep_running_; });
225+
updated_cond_.wait(lock_, [&] { return turn_ == State::NON_REALTIME || !keep_running_; });
218226
outgoing = msg_;
219227
}
220228

221229
// Sends the outgoing message
222230
if (keep_running_) {
223231
publisher_->publish(outgoing);
224232
}
225-
turn_.store(State::NON_REALTIME, std::memory_order_release);
226233
}
227234
is_running_ = false;
228235
}

0 commit comments

Comments
 (0)