Mir

Merge lp:~vanvugt/mir/fix-1338902 into lp:mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 1754
Proposed branch: lp:~vanvugt/mir/fix-1338902
Merge into: lp:mir
Diff against target: 39 lines (+9/-1)
1 file modified
tests/unit-tests/client/test_event_distributor.cpp (+9/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1338902
Reviewer Review Type Date Requested Status
Chris Halse Rogers Approve
PS Jenkins bot (community) continuous-integration Approve
Nick Dedekind Pending
Review via email: mp+225929@code.launchpad.net

Commit message

Fix crashing test: EventDistributorTest.succeeds_with_thread_delete_unregister
due to multiple mutex unlocks (LP: #1338902)

Also added a yield() to stop the test from taking forever under valgrind.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

This still seems to contain a race.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The failure doesn't seem related to the branch. Try again...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'tests/unit-tests/client/test_event_distributor.cpp'
--- tests/unit-tests/client/test_event_distributor.cpp 2014-06-11 16:11:32 +0000
+++ tests/unit-tests/client/test_event_distributor.cpp 2014-07-09 06:07:45 +0000
@@ -107,12 +107,18 @@
107 EventCatcher(mcl::EventDistributor* event_distributor)107 EventCatcher(mcl::EventDistributor* event_distributor)
108 : event_distributor(event_distributor)108 : event_distributor(event_distributor)
109 {109 {
110 locked = false;
110 reg = event_distributor->register_event_handler(111 reg = event_distributor->register_event_handler(
111 [this](MirEvent const&)112 [this](MirEvent const&)
112 {113 {
113 mutex.unlock();114 if (locked)
115 {
116 locked = false;
117 mutex.unlock();
118 }
114 });119 });
115 mutex.lock();120 mutex.lock();
121 locked = true;
116 }122 }
117 ~EventCatcher()123 ~EventCatcher()
118 {124 {
@@ -122,6 +128,7 @@
122128
123 mcl::EventDistributor* event_distributor;129 mcl::EventDistributor* event_distributor;
124 int reg;130 int reg;
131 bool locked;
125 std::mutex mutex;132 std::mutex mutex;
126 };133 };
127134
@@ -145,6 +152,7 @@
145152
146 while(!thread_done.woken()) {153 while(!thread_done.woken()) {
147 event_distributor.handle_event(e);154 event_distributor.handle_event(e);
155 std::this_thread::yield();
148 }156 }
149157
150 thread.join();158 thread.join();

Subscribers

People subscribed via source and target branches