Merge lp://qastaging/~afrantzis/mir/fix-1335819-surface-destruction-after-teardown into lp://qastaging/mir
| Status: | Rejected |
|---|---|
| Rejected by: | Cemil Azizoglu |
| Proposed branch: | lp://qastaging/~afrantzis/mir/fix-1335819-surface-destruction-after-teardown |
| Merge into: | lp://qastaging/mir |
| Diff against target: |
50 lines (+9/-4) 1 file modified
tests/acceptance-tests/test_client_surface_events.cpp (+9/-4) |
| To merge this branch: | bzr merge lp://qastaging/~afrantzis/mir/fix-1335819-surface-destruction-after-teardown |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| PS Jenkins bot (community) | continuous-integration | Approve | |
| Alberto Aguirre (community) | Abstain | ||
| Alan Griffiths | Needs Information | ||
|
Review via email:
|
|||
Commit message
tests: Don't keep surfaces alive and try to destroy them after the server has been torn down
The server is torn down during TearDown() but we keep a surface alive after that and try to destroy it when destroying the test fixture, leading to occasional crashes.
Description of the change
tests: Don't keep surfaces alive and try to destroy them after the server has been torn down
The server is torn down during TearDown() but we keep a surface alive after that and try to destroy it when destroying the test fixture, leading to occasional crashes.
It is debatable whether we should handle this case more gracefully in the server itself.
This solution is slightly more complicated than the alternative of reset()-ing the surface on TearDown(), but I think it communicates our intent to access (if possible) but not own the surface, and is the least intrusive in terms of how it affects object lifetimes.
| PS Jenkins bot (ps-jenkins) wrote | # |
Unmerged revisions
- 1732. By Alexandros Frantzis
-
tests: Don't keep surfaces alive and try to destroy them after the server has been torn down
*Needs Discussion*
I prefer to explicitly release the surface we grab in SetUp() in TearDown() rather than rely on its lifetime which is being controlled elsewhere (in the server implementation).
C.f. https:/