Fix race condition with preemption timers #307
Conversation
layus
force-pushed
the
fix-race-condition
branch
3 times, most recently
from
f3cd970 to
09fc238
Compare
layus
force-pushed
the
fix-race-condition
branch
from
09fc238 to
9eb2328
Compare
|
Is |
| @@ -50,7 +50,7 @@ BoostVM::BoostVM(BoostEnvironment& environment, | |||
| uuidGenerator(), | |||
| portClosed(false), | |||
| _asyncIONodeCount(0), | |||
| preemptionTimer(environment.io_service), | |||
| preemptionTimer(nullptr), | |||
| alarmTimer(environment.io_service), | |||
There was a problem hiding this comment.
What about this timer? Is it only accessed through this VM's thread?
There was a problem hiding this comment.
Yes. It would be simpler to do the same for preemptionTimer, but then it would not be able to restart itself, and the restart should be done in the vm when the preemption flag is cleared. And it looks difficult because the vm is a real vm, while the other is a boostvm...
There was a problem hiding this comment.
And it looks difficult because the vm is a real vm, while the other is a boostvm...
I think that just need adding one method in the interface between the two, but I don't remember the details. It would change semantics from "preempt every millisecond" to "preempt every millisecond+the time for VM to notice" though.
There was a problem hiding this comment.
Exactly. And this works, so why bother ;-)
Fixes #305 and closes #284.
/cc @sjrd, @eregon Would you mind reviewing this ?