Merge lp:~raof/mir/release-rendersurface-may-require-RPC into lp:mir
- release-rendersurface-may-require-RPC
- Merge into development-branch
Status: | Work in progress |
---|---|
Proposed branch: | lp:~raof/mir/release-rendersurface-may-require-RPC |
Merge into: | lp:mir |
Diff against target: |
601 lines (+262/-40) 14 files modified
include/test/mir/test/validity_matchers.h (+8/-0) playground/egldiamond_render_surface.c (+1/-1) playground/mir_demo_client_chain_jumping_buffers.c (+2/-1) playground/mir_demo_client_prerendered_frames.c (+1/-1) playground/render_surface.cpp (+1/-1) src/client/mir_connection.cpp (+14/-12) src/client/mir_connection.h (+3/-1) src/client/mir_render_surface_api.cpp (+31/-2) src/client/symbols.map (+1/-0) src/include/client/mir_toolkit/mir_render_surface.h (+13/-1) tests/acceptance-tests/staging/test_render_surface.cpp (+147/-16) tests/mir_test/CMakeLists.txt (+4/-0) tests/mir_test/validity_matchers.cpp (+31/-0) tests/unit-tests/client/test_mir_render_surface.cpp (+5/-4) |
To merge this branch: | bzr merge lp:~raof/mir/release-rendersurface-may-require-RPC |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Cemil Azizoglu (community) | Needs Fixing | ||
Mir CI Bot | continuous-integration | Approve | |
Review via email: mp+312761@code.launchpad.net |
Commit message
Rework mir_render_
In the case that the MirRenderSurface is backed by a MirBufferStream, releasing the MirRenderSurface
implies releasing the MirBufferStream, and releasing the MirBufferStream requires an RPC wait.
Other, future, MirRenderSurface backing objects might require RPC waits to release, too.
Split into the traditional mir_render_
Description of the change
Mir CI Bot (mir-ci-bot) wrote : | # |
- 3872. By Chris Halse Rogers
-
Fix MirRenderSurfac
eTest that make was mysteriously failing to rebuild
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3872
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Chris Halse Rogers (raof) wrote : | # |
Bug #1647573, retriggering.
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3872
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3872
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Cemil Azizoglu (cemil-azizoglu) wrote : | # |
123 + StreamRelease stream_
124 + nullptr,
125 + reinterpret_
126 + ctx,
127 rs->stream_
128 render_surface};
This is actually executed for PCs as well.
Cemil Azizoglu (cemil-azizoglu) wrote : | # |
I guess I don't understand why releasing of streams requires an RPC wait while that of chains doesn't, when the operation is the same (i.e. server.
Chris Halse Rogers (raof) wrote : | # |
Releasing streams requires an RPC wait for two reasons:
1) It's a necessary client synchronisation point: clients almost always want to have some data associated with a Mir object, and they can't free that associated data until we can guarantee they'll never get a callback on that Mir object again.
2) The code
mir_render_
mir_connection_
is currently racy, and will frequently lead to client deadlocks.
Like so:
Both the rs_release and connection_release client requests will get queued up
to the socket, the server sends the responses and then closes the connection.
.
The rs_release response results in erasing the bufferstream from the connection
map.
.
The bufferstream destructor calls release_buffer on all of its allocated buffers.
(This is why the current acceptance tests don't notice the problem - we don't
wait for the bufferstream to actually be valid, so its allocation requests
are unfulfilled at destruction time, so it never tries to release them.)
.
The buffer_release call hits the RPC layer, which notices that the socket has
been closed and fires on_disconnected.
.
on_disconnected runs all the pending completions. The first one is...
release_
lock, which we already hold way up in rs_release, and deadlock.
I guess both of these could be solved instead by
a) Ensuring that mir_render_
b) Ensuring that we don't deadlock in this scenario.
I'll see how difficult this is to do.
Chris Halse Rogers (raof) wrote : | # |
Hm. (2) is actually a flaw in my follow-up code, so it's only (1) that we're concerned about...
Unmerged revisions
- 3872. By Chris Halse Rogers
-
Fix MirRenderSurfac
eTest that make was mysteriously failing to rebuild - 3871. By Chris Halse Rogers
-
Rework mir_render_
surface_ release( ) to explicitly identify the RPC wait involved. In the case that the MirRenderSurface is backed by a MirBufferStream, releasing the MirRenderSurface
implies releasing the MirBufferStream, and releasing the MirBufferStream requires an RPC wait.Other, future, MirRenderSurface backing objects might require RPC waits to release, too.
Split into the traditional mir_render_
surface_ release( rs, callback, ctx)/_sync() pairs. - 3870. By Chris Halse Rogers
-
Add IsValid() implementations for MirBufferStream and MirRenderSurface
Preview Diff
1 | === modified file 'include/test/mir/test/validity_matchers.h' |
2 | --- include/test/mir/test/validity_matchers.h 2016-01-29 08:18:22 +0000 |
3 | +++ include/test/mir/test/validity_matchers.h 2016-12-08 03:24:08 +0000 |
4 | @@ -23,6 +23,8 @@ |
5 | |
6 | #include "mir_toolkit/mir_client_library.h" |
7 | |
8 | +class MirRenderSurface; |
9 | + |
10 | using ::testing::MakePolymorphicMatcher; |
11 | using ::testing::MatchResultListener; |
12 | using ::testing::NotNull; |
13 | @@ -54,6 +56,12 @@ |
14 | template<> |
15 | bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const; |
16 | |
17 | +template<> |
18 | +bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const; |
19 | + |
20 | +template<> |
21 | +bool IsValidMatcher::MatchAndExplain(MirRenderSurface* stream, MatchResultListener* listener) const; |
22 | + |
23 | // To construct a polymorphic matcher, pass an instance of the class |
24 | // to MakePolymorphicMatcher(). Note the return type. |
25 | inline PolymorphicMatcher<IsValidMatcher> IsValid() |
26 | |
27 | === modified file 'playground/egldiamond_render_surface.c' |
28 | --- playground/egldiamond_render_surface.c 2016-11-30 04:19:26 +0000 |
29 | +++ playground/egldiamond_render_surface.c 2016-12-08 03:24:08 +0000 |
30 | @@ -312,7 +312,7 @@ |
31 | future_driver_eglTerminate(egldisplay); |
32 | else |
33 | eglTerminate(egldisplay); |
34 | - mir_render_surface_release(render_surface); |
35 | + mir_render_surface_release_sync(render_surface); |
36 | mir_surface_release_sync(surface); |
37 | mir_connection_release(connection); |
38 | return 0; |
39 | |
40 | === modified file 'playground/mir_demo_client_chain_jumping_buffers.c' |
41 | --- playground/mir_demo_client_chain_jumping_buffers.c 2016-12-01 16:01:55 +0000 |
42 | +++ playground/mir_demo_client_chain_jumping_buffers.c 2016-12-08 03:24:08 +0000 |
43 | @@ -265,7 +265,8 @@ |
44 | for (unsigned int i = 0u; i < num_buffers; i++) |
45 | mir_buffer_release(buffer_available[i].buffer); |
46 | for (unsigned int i = 0u; i < num_chains; i++) |
47 | - mir_render_surface_release(render_surface[i]); |
48 | + mir_render_surface_release_sync(render_surface[i]); |
49 | + |
50 | mir_surface_release_sync(surface); |
51 | mir_connection_release(connection); |
52 | return 0; |
53 | |
54 | === modified file 'playground/mir_demo_client_prerendered_frames.c' |
55 | --- playground/mir_demo_client_prerendered_frames.c 2016-12-01 16:01:55 +0000 |
56 | +++ playground/mir_demo_client_prerendered_frames.c 2016-12-08 03:24:08 +0000 |
57 | @@ -232,7 +232,7 @@ |
58 | |
59 | for (i = 0u; i < num_prerendered_frames; i++) |
60 | mir_buffer_release(buffer_available[i].buffer); |
61 | - mir_render_surface_release(render_surface); |
62 | + mir_render_surface_release_sync(render_surface); |
63 | mir_surface_release_sync(surface); |
64 | mir_connection_release(connection); |
65 | return 0; |
66 | |
67 | === modified file 'playground/render_surface.cpp' |
68 | --- playground/render_surface.cpp 2016-11-14 22:39:13 +0000 |
69 | +++ playground/render_surface.cpp 2016-12-08 03:24:08 +0000 |
70 | @@ -243,7 +243,7 @@ |
71 | mir_buffer_stream_swap_buffers_sync(buffer_stream); |
72 | } |
73 | |
74 | - mir_render_surface_release(render_surface); |
75 | + mir_render_surface_release_sync(render_surface); |
76 | mir_surface_release_sync(surface); |
77 | close(signal_watch); |
78 | |
79 | |
80 | === modified file 'src/client/mir_connection.cpp' |
81 | --- src/client/mir_connection.cpp 2016-12-06 16:56:33 +0000 |
82 | +++ src/client/mir_connection.cpp 2016-12-08 03:24:08 +0000 |
83 | @@ -475,17 +475,17 @@ |
84 | |
85 | void MirConnection::released(StreamRelease data) |
86 | { |
87 | + { |
88 | + std::unique_lock<decltype(mutex)> lk(mutex); |
89 | + if (data.rs) |
90 | + surface_map->erase(data.rs); |
91 | + surface_map->erase(mf::BufferStreamId(data.rpc_id)); |
92 | + } |
93 | + |
94 | if (data.callback) |
95 | data.callback(data.stream, data.context); |
96 | if (data.handle) |
97 | data.handle->result_received(); |
98 | - |
99 | - { |
100 | - std::unique_lock<decltype(mutex)> lk(mutex); |
101 | - if (data.rs) |
102 | - surface_map->erase(data.rs); |
103 | - surface_map->erase(mf::BufferStreamId(data.rpc_id)); |
104 | - } |
105 | } |
106 | |
107 | void MirConnection::released(SurfaceRelease data) |
108 | @@ -1331,14 +1331,16 @@ |
109 | } |
110 | |
111 | void MirConnection::release_render_surface_with_content( |
112 | - void* render_surface) |
113 | + void* render_surface, |
114 | + mir_render_surface_callback callback, |
115 | + void* ctx) |
116 | { |
117 | auto rs = surface_map->render_surface(render_surface); |
118 | |
119 | - StreamRelease stream_release{nullptr, |
120 | - nullptr, |
121 | - nullptr, |
122 | - nullptr, |
123 | + StreamRelease stream_release{reinterpret_cast<MirBufferStream*>(render_surface), |
124 | + nullptr, |
125 | + reinterpret_cast<mir_buffer_stream_callback>(callback), |
126 | + ctx, |
127 | rs->stream_id().as_value(), |
128 | render_surface}; |
129 | |
130 | |
131 | === modified file 'src/client/mir_connection.h' |
132 | --- src/client/mir_connection.h 2016-12-06 16:56:33 +0000 |
133 | +++ src/client/mir_connection.h 2016-12-08 03:24:08 +0000 |
134 | @@ -224,7 +224,9 @@ |
135 | void* context, |
136 | void** native_window); |
137 | void release_render_surface_with_content( |
138 | - void* render_surface); |
139 | + void* render_surface, |
140 | + mir_render_surface_callback callback, |
141 | + void* ctx); |
142 | |
143 | void* request_interface(char const* name, int version); |
144 | |
145 | |
146 | === modified file 'src/client/mir_render_surface_api.cpp' |
147 | --- src/client/mir_render_surface_api.cpp 2016-12-06 16:56:33 +0000 |
148 | +++ src/client/mir_render_surface_api.cpp 2016-12-08 03:24:08 +0000 |
149 | @@ -158,19 +158,48 @@ |
150 | } |
151 | |
152 | void mir_render_surface_release( |
153 | - MirRenderSurface* render_surface) |
154 | + MirRenderSurface* render_surface, |
155 | + mir_render_surface_callback callback, |
156 | + void* ctx) |
157 | try |
158 | { |
159 | mir::require(render_surface); |
160 | auto connection = connection_map.connection(static_cast<void*>(render_surface)); |
161 | connection_map.erase(static_cast<void*>(render_surface)); |
162 | - connection->release_render_surface_with_content(render_surface); |
163 | + connection->release_render_surface_with_content(render_surface, callback, ctx); |
164 | } |
165 | catch (std::exception const& ex) |
166 | { |
167 | + callback(render_surface, ctx); |
168 | MIR_LOG_UNCAUGHT_EXCEPTION(ex); |
169 | } |
170 | |
171 | +void mir_render_surface_release_sync(MirRenderSurface* render_surface) |
172 | +{ |
173 | + struct Waiter |
174 | + { |
175 | + std::condition_variable notifier; |
176 | + std::mutex lock; |
177 | + bool done{false}; |
178 | + } waiter; |
179 | + |
180 | + mir_render_surface_release( |
181 | + render_surface, |
182 | + [](MirRenderSurface*, void* ctx) |
183 | + { |
184 | + auto waiter = reinterpret_cast<Waiter*>(ctx); |
185 | + { |
186 | + std::lock_guard<std::mutex> lock{waiter->lock}; |
187 | + waiter->done = true; |
188 | + } |
189 | + waiter->notifier.notify_all(); |
190 | + }, |
191 | + &waiter); |
192 | + |
193 | + std::unique_lock<std::mutex> lock(waiter.lock); |
194 | + waiter.notifier.wait(lock, [&waiter]() { return waiter.done; }); |
195 | +} |
196 | + |
197 | MirBufferStream* mir_render_surface_get_buffer_stream( |
198 | MirRenderSurface* render_surface, |
199 | int width, int height, |
200 | |
201 | === modified file 'src/client/symbols.map' |
202 | --- src/client/symbols.map 2016-12-06 16:56:33 +0000 |
203 | +++ src/client/symbols.map 2016-12-08 03:24:08 +0000 |
204 | @@ -396,6 +396,7 @@ |
205 | mir_render_surface_get_size; |
206 | mir_render_surface_set_size; |
207 | mir_render_surface_release; |
208 | + mir_render_surface_release_sync; |
209 | mir_surface_spec_add_render_surface; |
210 | |
211 | #private functions needed temporarily by nested passthrough |
212 | |
213 | === modified file 'src/include/client/mir_toolkit/mir_render_surface.h' |
214 | --- src/include/client/mir_toolkit/mir_render_surface.h 2016-12-06 16:56:33 +0000 |
215 | +++ src/include/client/mir_toolkit/mir_render_surface.h 2016-12-08 03:24:08 +0000 |
216 | @@ -110,9 +110,21 @@ |
217 | /** |
218 | * Release the specified render surface |
219 | * |
220 | - * \param [in] render_surface The render surface to be released |
221 | + * \param [in] render_surface The render surface to be released |
222 | + * \param [in] callback Callback to be run when the operation completes |
223 | + * \param [in,out] ctx Context passed to callback |
224 | */ |
225 | void mir_render_surface_release( |
226 | + MirRenderSurface* render_surface, |
227 | + mir_render_surface_callback callback, |
228 | + void* ctx); |
229 | + |
230 | +/** |
231 | + * Release the specified render surface and wait for the operation to complete |
232 | + * |
233 | + * \param [in] render_surface The render surface to be released |
234 | + */ |
235 | +void mir_render_surface_release_sync( |
236 | MirRenderSurface* render_surface); |
237 | |
238 | /** |
239 | |
240 | === modified file 'tests/acceptance-tests/staging/test_render_surface.cpp' |
241 | --- tests/acceptance-tests/staging/test_render_surface.cpp 2016-12-06 16:56:33 +0000 |
242 | +++ tests/acceptance-tests/staging/test_render_surface.cpp 2016-12-08 03:24:08 +0000 |
243 | @@ -24,11 +24,13 @@ |
244 | #include "mir_test_framework/headless_in_process_server.h" |
245 | #include "mir_test_framework/using_stub_client_platform.h" |
246 | #include "mir/test/validity_matchers.h" |
247 | +#include "mir/test/signal.h" |
248 | |
249 | #include <gtest/gtest.h> |
250 | #include <gmock/gmock.h> |
251 | |
252 | namespace mtf = mir_test_framework; |
253 | +namespace mt = mir::test; |
254 | namespace geom = mir::geometry; |
255 | |
256 | namespace |
257 | @@ -54,7 +56,7 @@ |
258 | EXPECT_TRUE(mir_render_surface_is_valid(rs)); |
259 | EXPECT_THAT(mir_render_surface_get_error_message(rs), StrEq("")); |
260 | |
261 | - mir_render_surface_release(rs); |
262 | + mir_render_surface_release_sync(rs); |
263 | mir_connection_release(connection); |
264 | } |
265 | |
266 | @@ -83,7 +85,7 @@ |
267 | EXPECT_TRUE(mir_buffer_stream_is_valid(bs)); |
268 | EXPECT_THAT(mir_buffer_stream_get_error_message(bs), StrEq("")); |
269 | |
270 | - mir_render_surface_release(rs); |
271 | + mir_render_surface_release_sync(rs); |
272 | mir_connection_release(connection); |
273 | } |
274 | |
275 | @@ -113,7 +115,7 @@ |
276 | |
277 | EXPECT_THAT(bs2, Eq(nullptr)); |
278 | |
279 | - mir_render_surface_release(rs); |
280 | + mir_render_surface_release_sync(rs); |
281 | mir_connection_release(connection); |
282 | } |
283 | |
284 | @@ -134,7 +136,7 @@ |
285 | EXPECT_TRUE(mir_buffer_stream_is_valid(bs)); |
286 | EXPECT_THAT(mir_buffer_stream_get_error_message(bs), StrEq("")); |
287 | |
288 | - mir_render_surface_release(rs); |
289 | + mir_render_surface_release_sync(rs); |
290 | mir_connection_release(connection); |
291 | } |
292 | |
293 | @@ -157,7 +159,7 @@ |
294 | EXPECT_THAT(surface, IsValid()); |
295 | EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr)); |
296 | |
297 | - mir_render_surface_release(rs); |
298 | + mir_render_surface_release_sync(rs); |
299 | mir_surface_release_sync(surface); |
300 | mir_connection_release(connection); |
301 | } |
302 | @@ -184,10 +186,11 @@ |
303 | usage); |
304 | |
305 | EXPECT_THAT(surface, IsValid()); |
306 | + |
307 | EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr)); |
308 | - EXPECT_TRUE(mir_buffer_stream_is_valid(bs)); |
309 | + EXPECT_THAT(bs, IsValid()); |
310 | |
311 | - mir_render_surface_release(rs); |
312 | + mir_render_surface_release_sync(rs); |
313 | mir_surface_release_sync(surface); |
314 | mir_connection_release(connection); |
315 | } |
316 | @@ -204,7 +207,7 @@ |
317 | ASSERT_THAT(pc, NotNull()); |
318 | EXPECT_TRUE(mir_presentation_chain_is_valid(pc)); |
319 | |
320 | - mir_render_surface_release(rs); |
321 | + mir_render_surface_release_sync(rs); |
322 | mir_connection_release(connection); |
323 | } |
324 | |
325 | @@ -224,7 +227,7 @@ |
326 | |
327 | EXPECT_THAT(pc2, Eq(nullptr)); |
328 | |
329 | - mir_render_surface_release(rs); |
330 | + mir_render_surface_release_sync(rs); |
331 | mir_connection_release(connection); |
332 | } |
333 | |
334 | @@ -249,7 +252,7 @@ |
335 | EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr)); |
336 | EXPECT_TRUE(mir_presentation_chain_is_valid(pc)); |
337 | |
338 | - mir_render_surface_release(rs); |
339 | + mir_render_surface_release_sync(rs); |
340 | mir_surface_release_sync(surface); |
341 | mir_connection_release(connection); |
342 | } |
343 | @@ -265,7 +268,7 @@ |
344 | ASSERT_THAT(pc, NotNull()); |
345 | EXPECT_TRUE(mir_presentation_chain_is_valid(pc)); |
346 | |
347 | - mir_render_surface_release(rs); |
348 | + mir_render_surface_release_sync(rs); |
349 | mir_connection_release(connection); |
350 | } |
351 | |
352 | @@ -289,7 +292,7 @@ |
353 | |
354 | EXPECT_THAT(bs, Eq(nullptr)); |
355 | |
356 | - mir_render_surface_release(rs); |
357 | + mir_render_surface_release_sync(rs); |
358 | mir_connection_release(connection); |
359 | } |
360 | |
361 | @@ -306,13 +309,141 @@ |
362 | mir_pixel_format_abgr_8888, |
363 | mir_buffer_usage_hardware); |
364 | |
365 | - ASSERT_THAT(bs, NotNull()); |
366 | - EXPECT_TRUE(mir_buffer_stream_is_valid(bs)); |
367 | + EXPECT_THAT(bs, IsValid()); |
368 | |
369 | auto pc = mir_render_surface_get_presentation_chain(rs); |
370 | |
371 | EXPECT_THAT(pc, Eq(nullptr)); |
372 | |
373 | - mir_render_surface_release(rs); |
374 | - mir_connection_release(connection); |
375 | + mir_render_surface_release_sync(rs); |
376 | + mir_connection_release(connection); |
377 | +} |
378 | + |
379 | +TEST_F(RenderSurfaceTest, asynchronous_render_surface_destruction_works_with_realised_buffer_stream) |
380 | +{ |
381 | + auto physical_size = logical_size; |
382 | + auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__); |
383 | + |
384 | + auto rs = mir_connection_create_render_surface_sync( |
385 | + connection, logical_size.width.as_int(), logical_size.height.as_int()); |
386 | + mir_render_surface_get_buffer_stream( |
387 | + rs, |
388 | + physical_size.width.as_int(), physical_size.height.as_int(), |
389 | + mir_pixel_format_abgr_8888, |
390 | + mir_buffer_usage_hardware); |
391 | + |
392 | + struct Context |
393 | + { |
394 | + MirRenderSurface* rs; |
395 | + mt::Signal done; |
396 | + } context; |
397 | + |
398 | + context.rs = rs; |
399 | + |
400 | + mir_render_surface_release( |
401 | + rs, |
402 | + [](MirRenderSurface* rs, void* ctx) |
403 | + { |
404 | + auto& context = *reinterpret_cast<Context*>(ctx); |
405 | + EXPECT_THAT(rs, Eq(context.rs)); |
406 | + /* |
407 | + * Should really test that the render surface isn't valid here, but that fails. |
408 | + * |
409 | + EXPECT_THAT(rs, Not(IsValid())); |
410 | + */ |
411 | + context.done.raise(); |
412 | + }, |
413 | + &context); |
414 | + |
415 | + ASSERT_TRUE(context.done.wait_for(std::chrono::seconds{20})); |
416 | + |
417 | + mir_connection_release(connection); |
418 | +} |
419 | + |
420 | +TEST_F(RenderSurfaceTest, asynchronous_render_surface_destruction_works_with_realised_presentation_chain) |
421 | +{ |
422 | + auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__); |
423 | + |
424 | + auto rs = mir_connection_create_render_surface_sync( |
425 | + connection, logical_size.width.as_int(), logical_size.height.as_int()); |
426 | + mir_render_surface_get_presentation_chain(rs); |
427 | + |
428 | + struct Context |
429 | + { |
430 | + MirRenderSurface* rs; |
431 | + mt::Signal done; |
432 | + } context; |
433 | + |
434 | + context.rs = rs; |
435 | + |
436 | + mir_render_surface_release( |
437 | + rs, |
438 | + [](MirRenderSurface* rs, void* ctx) |
439 | + { |
440 | + auto& context = *reinterpret_cast<Context*>(ctx); |
441 | + EXPECT_THAT(rs, Eq(context.rs)); |
442 | + /* |
443 | + * Should really test that the render surface isn't valid here, but that fails. |
444 | + * |
445 | + EXPECT_THAT(rs, Not(IsValid())); |
446 | + */ |
447 | + context.done.raise(); |
448 | + }, |
449 | + &context); |
450 | + |
451 | + ASSERT_TRUE(context.done.wait_for(std::chrono::seconds{20})); |
452 | + |
453 | + mir_connection_release(connection); |
454 | +} |
455 | + |
456 | +TEST_F(RenderSurfaceTest, asynchronous_render_surface_destruction_works_with_no_backing_object) |
457 | +{ |
458 | + auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__); |
459 | + |
460 | + auto rs = mir_connection_create_render_surface_sync( |
461 | + connection, logical_size.width.as_int(), logical_size.height.as_int()); |
462 | + |
463 | + struct Context |
464 | + { |
465 | + MirRenderSurface* rs; |
466 | + mt::Signal done; |
467 | + } context; |
468 | + |
469 | + context.rs = rs; |
470 | + |
471 | + mir_render_surface_release( |
472 | + rs, |
473 | + [](MirRenderSurface* rs, void* ctx) |
474 | + { |
475 | + auto& context = *reinterpret_cast<Context*>(ctx); |
476 | + EXPECT_THAT(rs, Eq(context.rs)); |
477 | + /* |
478 | + * Should really test that the render surface isn't valid here, but that fails. |
479 | + * |
480 | + EXPECT_THAT(rs, Not(IsValid())); |
481 | + */ |
482 | + context.done.raise(); |
483 | + }, |
484 | + &context); |
485 | + |
486 | + ASSERT_TRUE(context.done.wait_for(std::chrono::seconds{20})); |
487 | + |
488 | + mir_connection_release(connection); |
489 | +} |
490 | + |
491 | +TEST_F(RenderSurfaceTest, asynchronous_destruction_calls_callback_on_failure) |
492 | +{ |
493 | + auto not_a_valid_client_side_rs = reinterpret_cast<MirRenderSurface*>(0x0ff01bb1); |
494 | + |
495 | + mt::Signal callback_called; |
496 | + |
497 | + mir_render_surface_release( |
498 | + not_a_valid_client_side_rs, |
499 | + [](MirRenderSurface*, void* ctx) |
500 | + { |
501 | + reinterpret_cast<mt::Signal*>(ctx)->raise(); |
502 | + }, |
503 | + &callback_called); |
504 | + |
505 | + EXPECT_TRUE(callback_called.wait_for(std::chrono::seconds{20})); |
506 | } |
507 | |
508 | === modified file 'tests/mir_test/CMakeLists.txt' |
509 | --- tests/mir_test/CMakeLists.txt 2016-11-09 02:30:14 +0000 |
510 | +++ tests/mir_test/CMakeLists.txt 2016-12-08 03:24:08 +0000 |
511 | @@ -2,6 +2,10 @@ |
512 | string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS}) |
513 | string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS}) |
514 | |
515 | +include_directories( |
516 | + ${PROJECT_SOURCE_DIR}/src/include/client |
517 | +) |
518 | + |
519 | add_library(mir-public-test OBJECT |
520 | cross_process_action.cpp |
521 | cross_process_sync.cpp |
522 | |
523 | === modified file 'tests/mir_test/validity_matchers.cpp' |
524 | --- tests/mir_test/validity_matchers.cpp 2015-06-25 03:00:08 +0000 |
525 | +++ tests/mir_test/validity_matchers.cpp 2016-12-08 03:24:08 +0000 |
526 | @@ -1,5 +1,6 @@ |
527 | #include "mir/test/validity_matchers.h" |
528 | #include "mir_toolkit/mir_client_library.h" |
529 | +#include "mir_toolkit/mir_render_surface.h" |
530 | |
531 | template<> |
532 | bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const |
533 | @@ -30,3 +31,33 @@ |
534 | } |
535 | return true; |
536 | } |
537 | + |
538 | +template<> |
539 | +bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const |
540 | +{ |
541 | + if (stream == nullptr) |
542 | + { |
543 | + return false; |
544 | + } |
545 | + if (!mir_buffer_stream_is_valid(stream)) |
546 | + { |
547 | + *listener << "error is: " << mir_buffer_stream_get_error_message(stream); |
548 | + return false; |
549 | + } |
550 | + return true; |
551 | +} |
552 | + |
553 | +template<> |
554 | +bool IsValidMatcher::MatchAndExplain(MirRenderSurface* rs, MatchResultListener* listener) const |
555 | +{ |
556 | + if (rs == nullptr) |
557 | + { |
558 | + return false; |
559 | + } |
560 | + if (!mir_render_surface_is_valid(rs)) |
561 | + { |
562 | + *listener << "error is: " << mir_render_surface_get_error_message(rs); |
563 | + return false; |
564 | + } |
565 | + return true; |
566 | +} |
567 | |
568 | === modified file 'tests/unit-tests/client/test_mir_render_surface.cpp' |
569 | --- tests/unit-tests/client/test_mir_render_surface.cpp 2016-12-01 18:10:01 +0000 |
570 | +++ tests/unit-tests/client/test_mir_render_surface.cpp 2016-12-08 03:24:08 +0000 |
571 | @@ -44,10 +44,11 @@ |
572 | |
573 | namespace |
574 | { |
575 | -void assign_result(void* result, void** context) |
576 | +template<typename T> |
577 | +void assign_result(T* result, void* context) |
578 | { |
579 | if (context) |
580 | - *context = result; |
581 | + *reinterpret_cast<T**>(context) = result; |
582 | } |
583 | |
584 | struct RenderSurfaceCallback |
585 | @@ -208,14 +209,14 @@ |
586 | MirRenderSurface* render_surface = nullptr; |
587 | connection->create_render_surface_with_content( |
588 | {10, 10}, |
589 | - reinterpret_cast<mir_render_surface_callback>(assign_result), |
590 | + &assign_result, |
591 | &render_surface, |
592 | &nw); |
593 | |
594 | EXPECT_THAT(render_surface, NotNull()); |
595 | EXPECT_THAT(nw, NotNull()); |
596 | EXPECT_THAT(render_surface, Eq(nw)); |
597 | - EXPECT_NO_THROW(connection->release_render_surface_with_content(nw)); |
598 | + EXPECT_NO_THROW(connection->release_render_surface_with_content(nw, &assign_result, nullptr)); |
599 | |
600 | connection->disconnect(); |
601 | } |
FAILED: Continuous integration, rev:3871 /mir-jenkins. ubuntu. com/job/ mir-ci/ 2336/ /mir-jenkins. ubuntu. com/job/ build-mir/ 3050/console /mir-jenkins. ubuntu. com/job/ build-0- fetch/3116 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 3108 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial+ overlay/ 3108 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= yakkety/ 3108 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= yakkety/ 3079/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3079/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= yakkety/ 3079/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 3079/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 3079/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3079/console
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild: /mir-jenkins. ubuntu. com/job/ mir-ci/ 2336/rebuild
https:/