Skip to content

Commit 2427cf3

Browse files
committed
Fix crash at exit due to a race condition with new signal handler
Previously, the signal handler would interrupt the main thread (thread that calls `Server::Run`), so there was no chance for the main thread to resume before the signal handler was finished. With the new signal handler implementation in gz::common, the signal handler is now called from a different thread which means the main thread might resume before the signal handler is finished. This causes a race condition in which the `Server` class, and therefore the `ServerPrivate` class are being destructed while the signal handler is still active. Since `ServerPrivate` also contains the class that encapsulates the signal handler implementation, the causes a crash as the signal handler tries to access data that has been deleted. The fix here is to ensure that the main thread does not exit until the signal handler is finished by synchronizing on a mutex. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
1 parent 5298d46 commit 2427cf3

File tree

1 file changed

+16
-4
lines changed

1 file changed

+16
-4
lines changed

src/ServerPrivate.cc

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,20 @@ void ServerPrivate::OnSignal(int _sig)
123123
/////////////////////////////////////////////////
124124
void ServerPrivate::Stop()
125125
{
126-
this->running = false;
127-
for (std::unique_ptr<SimulationRunner> &runner : this->simRunners)
128-
{
129-
runner->Stop();
126+
// Stop might be called from the signal handler thread (new in Ionic) instead
127+
// of the main thread, so we need to ensure that we keep `ServerPrivate` alive
128+
// while the signal handler is still active. We do that by synchronizing on
129+
// the `runMutex` here in ServerPrivate::Run right after the call
130+
// SimulationRunner::Run returns. That way, `ServerPrivate::Run` cannot return
131+
// before the signal handler is finished.
132+
std::lock_guard<std::mutex> lock(this->runMutex);
133+
if (this->running)
134+
{
135+
this->running = false;
136+
for (std::unique_ptr<SimulationRunner> &runner : this->simRunners)
137+
{
138+
runner->Stop();
139+
}
130140
}
131141
}
132142

@@ -196,6 +206,8 @@ bool ServerPrivate::Run(const uint64_t _iterations,
196206
result = this->workerPool.WaitForResults();
197207
}
198208

209+
// See comments ServerPrivate::Stop() for why we lock this mutex here.
210+
std::lock_guard<std::mutex> lock(this->runMutex);
199211
this->running = false;
200212
return result;
201213
}

0 commit comments

Comments
 (0)