Skip to content

Commit 9eb2328

Browse files
committed
Fix race condition with preemption timers
1 parent 001e414 commit 9eb2328

File tree

2 files changed

+41
-15
lines changed

2 files changed

+41
-15
lines changed

vm/boostenv/main/boostvm-decl.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ private:
167167

168168
// Preemption and alarms
169169
private:
170-
boost::asio::deadline_timer preemptionTimer;
170+
boost::asio::deadline_timer* preemptionTimer;
171171
boost::asio::deadline_timer alarmTimer;
172172

173173
// IO-driven events that must work with the VM store

vm/boostenv/main/boostvm.cc

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ BoostVM::BoostVM(BoostEnvironment& environment,
5050
uuidGenerator(),
5151
portClosed(false),
5252
_asyncIONodeCount(0),
53-
preemptionTimer(environment.io_service),
53+
preemptionTimer(nullptr),
5454
alarmTimer(environment.io_service),
5555
_terminationRequested(false),
5656
_terminationStatus(0),
@@ -103,23 +103,31 @@ void BoostVM::run() {
103103
constexpr auto recInvokeAgainNow = VirtualMachine::recInvokeAgainNow;
104104
constexpr auto recInvokeAgainLater = VirtualMachine::recInvokeAgainLater;
105105

106+
env.io_service.post([&] {
107+
preemptionTimer = new boost::asio::deadline_timer(env.io_service);
108+
});
109+
106110
// The main loop that handles all interactions with the VM
107111
while (true) {
108112
// Make sure the VM knows the reference time before starting
109113
vm->setReferenceTime(env.getReferenceTime());
110114

111115
// Setup the preemption timer
112-
preemptionTimer.expires_from_now(boost::posix_time::millisec(1));
113-
preemptionTimer.async_wait(boost::bind(
114-
&BoostVM::onPreemptionTimerExpire,
115-
this, boost::asio::placeholders::error));
116+
env.io_service.post([&](){
117+
preemptionTimer->expires_from_now(boost::posix_time::millisec(1));
118+
preemptionTimer->async_wait(boost::bind(
119+
&BoostVM::onPreemptionTimerExpire,
120+
this, boost::asio::placeholders::error));
121+
});
116122

117123
// Run the VM
118124
auto nextInvokePair = vm->run();
119125
auto nextInvoke = nextInvokePair.first;
120126

121127
// Stop the preemption timer
122-
preemptionTimer.cancel();
128+
env.io_service.post([&](){
129+
preemptionTimer->expires_at(boost::posix_time::min_date_time);
130+
});
123131

124132
{
125133
// Acquire the lock that grants me access to
@@ -170,18 +178,23 @@ void BoostVM::run() {
170178

171179
// Called by the *IO thread*
172180
void BoostVM::onPreemptionTimerExpire(const boost::system::error_code& error) {
173-
if (error != boost::asio::error::operation_aborted &&
174-
!_terminationRequested) {
181+
if (error == boost::asio::error::operation_aborted) {
182+
// Timer was cancelled
183+
} else if (_terminationRequested) {
184+
// Termination was requested
185+
} else if (preemptionTimer->expires_at() == boost::posix_time::min_date_time) {
186+
// Timer was cancelled, but we missed it (race condition in io_service)
187+
} else {
175188
// Preemption
176189
vm->setReferenceTime(env.getReferenceTime());
177190
vm->requestPreempt();
178191

179192
// Reschedule
180-
preemptionTimer.expires_at(
181-
preemptionTimer.expires_at() + boost::posix_time::millisec(1));
182-
preemptionTimer.async_wait(boost::bind(
183-
&BoostVM::onPreemptionTimerExpire,
184-
this, boost::asio::placeholders::error));
193+
preemptionTimer->expires_at(
194+
preemptionTimer->expires_at() + boost::posix_time::millisec(1));
195+
preemptionTimer->async_wait(boost::bind(
196+
&BoostVM::onPreemptionTimerExpire,
197+
this, boost::asio::placeholders::error));
185198
}
186199
}
187200

@@ -320,7 +333,20 @@ void BoostVM::terminate() {
320333
// not interact with the VM as we might have quitted run() brutally.
321334

322335
// Ensure the timers are stopped
323-
preemptionTimer.cancel();
336+
auto& preemptionTimerCopy = preemptionTimer;
337+
// We need a copy of preemptionTimer because we cannot capture 'this'.
338+
// It may be deleted before the execution of the callback.
339+
// For the same reason, we access the io_service via the timer.
340+
env.io_service.post([preemptionTimerCopy]{
341+
preemptionTimerCopy->expires_at(boost::posix_time::min_date_time);
342+
// We cannot delete the timer now because the onPreemptionTimerExpire handler
343+
// may already be in the queue. So add a delete lambda to the queue.
344+
// The lambda will execute after any leftover handlers on the timer and
345+
// with the timer stopped.
346+
preemptionTimerCopy->get_io_service().post([preemptionTimerCopy] {
347+
delete preemptionTimerCopy;
348+
});
349+
});
324350
alarmTimer.cancel();
325351

326352
portClosed = true; // close VM port

0 commit comments

Comments
 (0)