Merge lp://staging/~albaguirre/mir/fix-1522105 into lp://staging/mir
Proposed by
Alberto Aguirre
| Status: | Rejected |
|---|---|
| Rejected by: | Alberto Aguirre |
| Proposed branch: | lp://staging/~albaguirre/mir/fix-1522105 |
| Merge into: | lp://staging/mir |
| Diff against target: |
253 lines (+87/-24) 3 files modified
src/server/scene/surface_stack.cpp (+15/-9) src/server/scene/surface_stack.h (+5/-4) tests/unit-tests/scene/test_surface_stack.cpp (+67/-11) |
| To merge this branch: | bzr merge lp://staging/~albaguirre/mir/fix-1522105 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Alan Griffiths | Abstain | ||
| PS Jenkins bot (community) | continuous-integration | Approve | |
|
Review via email:
|
|||
Commit message
Fix SurfaceStack:
Another thread could call SurfaceStack:
A secondary lock is used to ensure surface_removed and add_observer are mutually exclusive thus ensuring the order of calls to Observer:
Description of the change
A bit ugly but fixes the bug. Note that a surface_exists handler which triggers another thread to remove a surface from the scene could potentially deadlock - but I fail to a reason we should support that.
To post a comment you must log in.
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | # |
review:
Approve
(continuous-integration)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Clearly the "before" is worse than the proposal - it holds (and uses) a reference to an element that needs to be locked to avoid invalidation.
While there are no obvious problems with the proposed approach it isn't obvious that there are no problems. Maybe this is a case for RecursiveReadWr