MirWaitHandle is unreliable if reused in some cases
Bug #1151645 reported by
Daniel van Vugt
This bug affects 1 person
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Mir |
Fix Released
|
High
|
Daniel van Vugt | ||
Bug Description
MirWaitHandle is unreliable if reused in some cases. The problem can be seen if you add a new client function mir_foo and it gets called like:
mir_foo(x, A);
mir_foo(x, B);
mir_
where mir_foo always reuses and returns the same MirWaitHandle for x.
The problem is that after mir_wait_for returns, usually only A or B has completed. Not C. And having:
mir_
return before C is completed is dangerously counter-intuitive.
We could avoid the problem if each call to mir_foo returns a different MirWaitHandle, but that's probably very hard to do reliably in C without leaking.
A full integration test demonstrating the problem is coming soon in the surface-types branch.
Related branches
lp://qastaging/~vanvugt/mir/fix-1151645
- Robert Carr (community): Approve
- Alan Griffiths: Approve
- PS Jenkins bot (community): Approve (continuous-integration)
-
Diff: 216 lines (+135/-6)4 files modifiedsrc/client/mir_wait_handle.cpp (+16/-5)
src/client/mir_wait_handle.h (+4/-1)
tests/unit-tests/client/CMakeLists.txt (+1/-0)
tests/unit-tests/client/test_wait_handle.cpp (+114/-0)
Revision history for this message
| Michi Henning (michihenning) wrote | #1 |
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | #2 |
| Changed in mir: | |
| status: | In Progress → Fix Committed |
| Changed in mir: | |
| milestone: | none → 0.0.3 |
| Changed in mir: | |
| status: | Fix Committed → Fix Released |
To post a comment you must log in.
If an async invocation returns the same handle for two different invocations, it's broken.
The whole point of having async calls return a token that identifies this particular call is to make sure that *all* of the state to complete an async invocation is encapsulated in that token. If that is not the case, I can no longer reliable distinguish the state for outstanding async invocations.
We had a similar problem in the old API for async invocations in Ice. There were scenarios were it was possible to accidentally reuse an async handle/object while a previous invocation was still in flight, causing the Ice run time to get confused.
In short, the solution to the problem is to encapsulate *all* of the state that is required to complete an async invocation in the start/begin/launch method of that invocation. There must be no state that persists in the run time in between the start and the end call, otherwise there will be problems down the track.