SessionMediator locks mutexes in one thread and unlocks them in another
Bug #1427976 reported by
Alberto Aguirre
This bug affects 1 person
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Mir |
Fix Released
|
High
|
Alberto Aguirre | ||
| 0.12 |
Fix Released
|
High
|
Alberto Aguirre | ||
| mir (Ubuntu) |
Fix Released
|
High
|
Unassigned | ||
Bug Description
SessionMediator locks mutexes in one thread and unlocks them in another thread during create_surface or exchange_buffer. This can cause undefined behavior:
http://
"If the mutex is not currently locked by the calling thread, it causes undefined behavior."
This may potentially be the root cause of a hang (wait handle never signals) seen when creating/destroying menu surfaces while a parent does swap_buffers.
Related branches
lp://qastaging/~albaguirre/mir/fix-1427976
- Alberto Aguirre (community): Disapprove
- Alan Griffiths: Needs Fixing
- Alexandros Frantzis (community): Needs Fixing
- Daniel van Vugt: Needs Fixing
- PS Jenkins bot (community): Approve (continuous-integration)
-
Diff: 363 lines (+162/-23)5 files modifiedsrc/include/common/mir/semaphore_lock.h (+81/-0)
src/server/frontend/session_mediator.cpp (+22/-22)
src/server/frontend/session_mediator.h (+2/-1)
tests/unit-tests/CMakeLists.txt (+1/-0)
tests/unit-tests/test_semaphore_lock.cpp (+56/-0)
lp://qastaging/~albaguirre/mir/fix-1427976-take2
- Alexandros Frantzis (community): Approve
- PS Jenkins bot (community): Approve (continuous-integration)
- Alan Griffiths: Approve
-
Diff: 232 lines (+41/-30)4 files modifiedsrc/server/frontend/session_mediator.cpp (+25/-26)
src/server/frontend/session_mediator.h (+2/-0)
src/server/frontend/surface_tracker.cpp (+8/-1)
src/server/frontend/surface_tracker.h (+6/-3)
lp://qastaging/mir/0.12
- PS Jenkins bot (community): Needs Fixing (continuous-integration)
- Mir development team: Pending requested
-
Diff: 2349 lines (+1697/-127)23 files modified3rd_party/android-input/android/frameworks/base/services/input/InputDispatcher.cpp (+36/-31)
cmake/MirCommon.cmake (+2/-0)
debian/changelog (+22/-0)
src/CMakeLists.txt (+1/-1)
src/platforms/android/server/hwc_blanking_control.cpp (+14/-1)
src/platforms/mesa/server/cursor.cpp (+31/-3)
src/server/frontend/protobuf_message_processor.cpp (+42/-15)
src/server/frontend/protobuf_message_processor.h (+2/-1)
src/server/frontend/protobuf_responder.cpp (+9/-5)
src/server/frontend/protobuf_responder.h (+2/-0)
src/server/frontend/session_mediator.cpp (+25/-26)
src/server/frontend/session_mediator.h (+2/-0)
src/server/frontend/surface_tracker.cpp (+8/-1)
src/server/frontend/surface_tracker.h (+6/-3)
src/server/graphics/software_cursor.cpp (+8/-20)
src/server/graphics/software_cursor.h (+1/-1)
tests/acceptance-tests/throwback/test_client_input.cpp (+3/-2)
tests/unit-tests/frontend/test_protobuf_message_processor.cpp (+1/-1)
tests/unit-tests/graphics/android/test_hwc_configuration.cpp (+28/-13)
tests/unit-tests/graphics/test_software_cursor.cpp (+20/-0)
tests/unit-tests/test_fatal.cpp (+3/-3)
tools/valgrind_suppressions_armhf (+27/-0)
tools/valgrind_suppressions_glibc_2.21 (+1404/-0)
Revision history for this message
| Daniel van Vugt (vanvugt) wrote | #1 |
Revision history for this message
| Cemil Azizoglu (cemil-azizoglu) wrote | #2 |
| Changed in mir: | |
| importance: | Undecided → High |
| Changed in mir: | |
| status: | New → In Progress |
Revision history for this message
| Alberto Aguirre (albaguirre) wrote | #3 |
Revision history for this message
| Alberto Aguirre (albaguirre) wrote | #4 |
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote | #5 |
| Changed in mir: | |
| status: | In Progress → Fix Committed |
| Changed in mir (Ubuntu): | |
| importance: | Undecided → High |
| status: | New → Triaged |
Revision history for this message
| Launchpad Janitor (janitor) wrote | #6 |
| Changed in mir (Ubuntu): | |
| status: | Triaged → Fix Released |
| Changed in mir: | |
| status: | Fix Committed → Fix Released |
To post a comment you must log in.
I noticed this when hacking exchange_buffer in the past. It always looked concerning to me.
But the question is not whether a locked mutex is portable between threads (because it's not). The question is rather: is the implementation of the *lock template portable between threads? It might be...