Mir

Merge lp:~afrantzis/mir/fix-1378740 into lp:mir

Proposed by Alexandros Frantzis on 2014-10-08
Status: Merged
Approved by: Alexandros Frantzis on 2014-10-09
Approved revision: 1968
Merged at revision: 1969
Proposed branch: lp:~afrantzis/mir/fix-1378740
Merge into: lp:mir
Diff against target: 210 lines (+104/-20)
6 files modified
src/server/input/android/android_input_dispatcher.cpp (+7/-0)
src/server/input/android/android_input_dispatcher.h (+1/-0)
src/server/input/android/android_input_manager.cpp (+3/-0)
tests/integration-tests/test_server_shutdown.cpp (+38/-0)
tests/unit-tests/input/android/test_android_input_dispatcher.cpp (+15/-1)
tests/unit-tests/input/android/test_android_input_manager.cpp (+40/-19)
To merge this branch: bzr merge lp:~afrantzis/mir/fix-1378740
Reviewer Review Type Date Requested Status
Andreas Pokorny (community) Approve on 2014-10-08
Alan Griffiths 2014-10-08 Approve on 2014-10-08
PS Jenkins bot (community) continuous-integration Approve on 2014-10-08
Review via email: mp+237580@code.launchpad.net

Commit message

input: Stop input threads properly when their controlling components get destroyed (LP: #1378740)

Otherwise we may destroy the thread objects without first having joined (or detached) them, leading to an abort.

Description of the change

input: Stop input threads properly when their controlling components get destroyed (LP: #1378740)

Otherwise we may destroy the thread objects without first having joined (or detached) them, leading to an abort.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/input/android/android_input_dispatcher.cpp'
2--- src/server/input/android/android_input_dispatcher.cpp 2014-09-11 05:51:44 +0000
3+++ src/server/input/android/android_input_dispatcher.cpp 2014-10-08 11:35:30 +0000
4@@ -37,6 +37,13 @@
5 {
6 }
7
8+mia::AndroidInputDispatcher::~AndroidInputDispatcher()
9+{
10+ // It is safe to call stop(), even if we haven't been started at all,
11+ // or we have been previously started and stopped manually.
12+ stop();
13+}
14+
15 void mia::AndroidInputDispatcher::configuration_changed(nsecs_t when)
16 {
17 droidinput::NotifyConfigurationChangedArgs args(when);
18
19=== modified file 'src/server/input/android/android_input_dispatcher.h'
20--- src/server/input/android/android_input_dispatcher.h 2014-09-11 05:51:44 +0000
21+++ src/server/input/android/android_input_dispatcher.h 2014-10-08 11:35:30 +0000
22@@ -45,6 +45,7 @@
23 public:
24 AndroidInputDispatcher(std::shared_ptr<droidinput::InputDispatcherInterface> const& dispatcher,
25 std::shared_ptr<InputThread> const& thread);
26+ ~AndroidInputDispatcher();
27 void configuration_changed(nsecs_t when) override;
28 void device_reset(int32_t device_id, nsecs_t when) override;
29 void dispatch(MirEvent const& event) override;
30
31=== modified file 'src/server/input/android/android_input_manager.cpp'
32--- src/server/input/android/android_input_manager.cpp 2014-09-11 05:51:44 +0000
33+++ src/server/input/android/android_input_manager.cpp 2014-10-08 11:35:30 +0000
34@@ -36,6 +36,9 @@
35
36 mia::InputManager::~InputManager()
37 {
38+ // It is safe to call stop(), even if we haven't been started at all,
39+ // or we have been previously started and stopped manually.
40+ stop();
41 }
42
43 void mia::InputManager::stop()
44
45=== modified file 'tests/integration-tests/test_server_shutdown.cpp'
46--- tests/integration-tests/test_server_shutdown.cpp 2014-09-30 13:50:09 +0000
47+++ tests/integration-tests/test_server_shutdown.cpp 2014-10-08 11:35:30 +0000
48@@ -22,6 +22,7 @@
49 #include "mir/compositor/renderer_factory.h"
50
51 #include "mir/run_mir.h"
52+#include "mir/main_loop.h"
53
54 #include "mir_toolkit/mir_client_library.h"
55
56@@ -432,3 +433,40 @@
57 EXPECT_EQ(0, connector.use_count());
58 EXPECT_EQ(0, input_manager.use_count());
59 }
60+
61+// This also acts as a regression test for LP: #1378740
62+TEST(ServerShutdownWithThreadException,
63+ server_releases_resources_on_abnormal_main_thread_termination)
64+{
65+ // Use the FakeEventHubServerConfig to get the production input components
66+ // (with the exception of EventHub, of course).
67+ auto server_config = std::make_shared<FakeEventHubServerConfig>();
68+
69+ std::thread server{
70+ [&]
71+ {
72+ EXPECT_THROW(
73+ mir::run_mir(*server_config,
74+ [server_config](mir::DisplayServer&)
75+ {
76+ server_config->the_main_loop()->enqueue(
77+ server_config.get(),
78+ [] { throw std::runtime_error(""); });
79+ }),
80+ std::runtime_error);
81+ }};
82+
83+ server.join();
84+
85+ std::weak_ptr<mir::graphics::Display> display = server_config->the_display();
86+ std::weak_ptr<mir::compositor::Compositor> compositor = server_config->the_compositor();
87+ std::weak_ptr<mir::frontend::Connector> connector = server_config->the_connector();
88+ std::weak_ptr<mir::input::InputManager> input_manager = server_config->the_input_manager();
89+
90+ server_config.reset();
91+
92+ EXPECT_EQ(0, display.use_count());
93+ EXPECT_EQ(0, compositor.use_count());
94+ EXPECT_EQ(0, connector.use_count());
95+ EXPECT_EQ(0, input_manager.use_count());
96+}
97
98=== modified file 'tests/unit-tests/input/android/test_android_input_dispatcher.cpp'
99--- tests/unit-tests/input/android/test_android_input_dispatcher.cpp 2014-09-11 05:51:44 +0000
100+++ tests/unit-tests/input/android/test_android_input_dispatcher.cpp 2014-10-08 11:35:30 +0000
101@@ -57,6 +57,20 @@
102
103 struct AndroidInputDispatcherTest : public testing::Test
104 {
105+ ~AndroidInputDispatcherTest()
106+ {
107+ using namespace ::testing;
108+
109+ Mock::VerifyAndClearExpectations(dispatcher.get());
110+ Mock::VerifyAndClearExpectations(dispatcher_thread.get());
111+
112+ // mia::AndroidInputDispatcher destruction expectations
113+ InSequence s;
114+ EXPECT_CALL(*dispatcher_thread, request_stop());
115+ EXPECT_CALL(*dispatcher, setInputDispatchMode(mia::DispatchDisabled, mia::DispatchFrozen));
116+ EXPECT_CALL(*dispatcher_thread, join());
117+ }
118+
119 std::shared_ptr<mtd::MockAndroidInputDispatcher> dispatcher = std::make_shared<mtd::MockAndroidInputDispatcher>();
120 std::shared_ptr<MockInputThread> dispatcher_thread = std::make_shared<MockInputThread>();
121 mia::AndroidInputDispatcher input_dispatcher{dispatcher,dispatcher_thread};
122@@ -65,7 +79,7 @@
123
124 }
125
126-TEST_F(AndroidInputDispatcherTest, start_starts_dispathcer_and_thread)
127+TEST_F(AndroidInputDispatcherTest, start_starts_dispatcher_and_thread)
128 {
129 using namespace ::testing;
130
131
132=== modified file 'tests/unit-tests/input/android/test_android_input_manager.cpp'
133--- tests/unit-tests/input/android/test_android_input_manager.cpp 2014-09-11 05:51:44 +0000
134+++ tests/unit-tests/input/android/test_android_input_manager.cpp 2014-10-08 11:35:30 +0000
135@@ -95,35 +95,56 @@
136 {
137 std::shared_ptr<MockEventHub> event_hub = std::make_shared<MockEventHub>();
138 std::shared_ptr<MockInputThread> reader_thread = std::make_shared<MockInputThread>();
139-};
140-
141-}
142-
143-TEST_F(AndroidInputManagerSetup, start_and_stop)
144-{
145- using namespace ::testing;
146-
147- ExpectationSet reader_setup;
148-
149-
150- reader_setup += EXPECT_CALL(*event_hub, flush()).Times(1);
151-
152- EXPECT_CALL(*reader_thread, start())
153- .Times(1)
154- .After(reader_setup);
155-
156- {
157- InSequence seq;
158+
159+ void setup_start_expectations()
160+ {
161+ testing::InSequence seq;
162+
163+ EXPECT_CALL(*event_hub, flush());
164+ EXPECT_CALL(*reader_thread, start());
165+ }
166+
167+ void setup_stop_expectations()
168+ {
169+ testing::InSequence seq;
170
171 EXPECT_CALL(*reader_thread, request_stop());
172 EXPECT_CALL(*event_hub, wake());
173 EXPECT_CALL(*reader_thread, join());
174 }
175+};
176+
177+}
178+
179+TEST_F(AndroidInputManagerSetup, starts_and_stops_reader)
180+{
181+ using namespace ::testing;
182+
183+ setup_start_expectations();
184+ setup_stop_expectations();
185
186 mia::InputManager manager(event_hub, reader_thread);
187
188 manager.start();
189 manager.stop();
190+
191+ // The input manager is unconditionally stopped at destruction.
192+ Mock::VerifyAndClearExpectations(reader_thread.get());
193+ Mock::VerifyAndClearExpectations(event_hub.get());
194+
195+ setup_stop_expectations();
196+}
197+
198+TEST_F(AndroidInputManagerSetup, stops_reader_at_destruction)
199+{
200+ using namespace ::testing;
201+
202+ setup_start_expectations();
203+ setup_stop_expectations();
204+
205+ mia::InputManager manager(event_hub, reader_thread);
206+
207+ manager.start();
208 }
209
210 TEST_F(AndroidInputManagerSetup, channel_factory_returns_input_channel_with_fds)

Subscribers

People subscribed via source and target branches