On Thu, 28 Apr 2016 15:31:32 +0800 Jonas Ådahl <[email protected]> wrote:
> Test that doing wl_display.sync on a wrapped proxy with a special queue > works as expected. > > Test that creating a wrapper on a destroyed but not freed proxy fails. > > Signed-off-by: Jonas Ådahl <[email protected]> > --- > > Changes since v1: > > - Made the test single threaded, instead pretending to be multi threaded. > - Added a negative-test for illustrating how NOT to do thread safe queue > assignment. > - Changed create-wrapper-failed test to be less confusing > > Jonas Hi Jonas, much appreciated. :-) > tests/queue-test.c | 157 > +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 157 insertions(+) > > diff --git a/tests/queue-test.c b/tests/queue-test.c > index 02865ae..a6f82ac 100644 > --- a/tests/queue-test.c > +++ b/tests/queue-test.c > @@ -205,6 +205,127 @@ client_test_queue_roundtrip(void) > } > > static void > +client_test_queue_proxy_wrapper(void) > +{ > + struct wl_event_queue *queue; > + struct wl_display *display; > + struct wl_display *display_wrapper; > + struct wl_callback *callback; > + bool done = false; > + > + /* > + * For an illustration of what usage would normally fail without using > + * proxy wrappers, see the `client_test_queue_set_queue_race' test case. > + */ > + > + display = wl_display_connect(NULL); > + assert(display); > + > + /* Pretend we are in a separate thread where a thread-local queue is > + * used. */ > + queue = wl_display_create_queue(display); > + assert(queue); > + > + display_wrapper = wl_proxy_create_wrapper(display); > + assert(display_wrapper); > + wl_proxy_set_queue((struct wl_proxy *) display_wrapper, queue); > + callback = wl_display_sync(display_wrapper); > + wl_proxy_wrapper_destroy(display_wrapper); > + assert(callback != NULL); > + > + /* Pretend we are now another thread and dispatch the dispatch the main -"dispatch the" > + * queue while also knowing our callback is read and queued. */ > + wl_display_roundtrip(display); > + > + /* Make sure that the pretend-to-be main thread didn't dispatch our > + * callback, behind our back. */ > + wl_callback_add_listener(callback, &sync_listener_roundtrip, &done); > + wl_display_flush(display); Is this flush needed for something? > + > + assert(!done); > + > + /* Make sure that we eventually end up dispatching our callback. */ > + while (!done) > + assert(wl_display_dispatch_queue(display, queue) != -1); > + > + wl_callback_destroy(callback); > + wl_event_queue_destroy(queue); > + > + wl_display_disconnect(display); > +} > + > +static void > +client_test_queue_set_queue_race(void) > +{ > + struct wl_event_queue *queue; > + struct wl_display *display; > + struct wl_callback *callback; > + bool done = false; > + > + /* > + * This test illustrates the multi threading scenario which would fail > + * without doing what is done in the `client_test_queue_proxy_wrapper' > + * test. > + */ I think this is crucial for understanding what the positive test is testing for. Thank you for adding this. :-) > + > + display = wl_display_connect(NULL); > + assert(display); > + > + /* Pretend we are in a separate thread where a thread-local queue is > + * used. */ > + queue = wl_display_create_queue(display); > + assert(queue); > + > + callback = wl_display_sync(display); > + assert(callback != NULL); > + > + /* Pretend we are now another thread and dispatch the dispatch the main -"dispatch the" > + * queue while also knowing our callback is read, queued on the wrong > + * queue, and dispatched. */ > + wl_display_roundtrip(display); > + > + /* Pretend we are back in the separate thread, and continue with setting > + * up our callback. */ > + wl_callback_add_listener(callback, &sync_listener_roundtrip, &done); > + wl_proxy_set_queue((struct wl_proxy *) callback, queue); > + > + /* Roundtrip our separate thread queue to make sure any events are > + * dispatched. */ > + wl_display_roundtrip_queue(display, queue); > + > + /* Verify that the callback has indeed been dropped. */ > + assert(!done); > + > + wl_callback_destroy(callback); > + wl_event_queue_destroy(queue); > + > + wl_display_disconnect(display); > +} > + > +static void > +client_test_queue_wrap_destroyed_proxy(void) > +{ > + struct wl_display *display; > + struct wl_callback *callback; > + struct wl_callback *callback_wrapper; > + > + display = wl_display_connect(NULL); > + assert(display); > + > + callback = wl_display_sync(display); > + assert(callback); > + > + wl_display_roundtrip(display); > + > + /* The callback will now been removed. */ > + callback_wrapper = wl_proxy_create_wrapper(callback); > + assert(!callback_wrapper); > + > + wl_callback_destroy(callback); > + wl_display_disconnect(display); > +} Yes, this is exactly what I meant! > + > +static void > dummy_bind(struct wl_client *client, > void *data, uint32_t version, uint32_t id) > { > @@ -259,3 +380,39 @@ TEST(queue_roundtrip) > > display_destroy(d); > } > + > +TEST(queue_set_queue_proxy_wrapper) > +{ > + struct display *d = display_create(); > + > + test_set_timeout(2); > + > + client_create_noarg(d, client_test_queue_proxy_wrapper); > + display_run(d); > + > + display_destroy(d); > +} > + > +TEST(queue_set_queue_race) > +{ > + struct display *d = display_create(); > + > + test_set_timeout(2); > + > + client_create_noarg(d, client_test_queue_set_queue_race); > + display_run(d); > + > + display_destroy(d); > +} > + > +TEST(queue_wrap_destroyed_proxy) > +{ > + struct display *d = display_create(); > + > + test_set_timeout(2); > + > + client_create_noarg(d, client_test_queue_wrap_destroyed_proxy); > + display_run(d); > + > + display_destroy(d); > +} Apart from few cosmetic issues, excellent. Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq
pgpLhQQZeRbAZ.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
