<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 21 August 2014 15:15, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">On Wed, 25 Jun 2014 14:35:16 +0200<br>
Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> wrote:<br>
<br>
> Test if events are going to the right queue and if the queue<br>
> is interrupted from polling when an error to the main queue comes.<br>
> The last one is failing.<br>
> ---<br>
> tests/queue-test.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++<br>
> 1 file changed, 128 insertions(+)<br>
><br>
> diff --git a/tests/queue-test.c b/tests/queue-test.c<br>
> index 36d7a69..307bae9 100644<br>
> --- a/tests/queue-test.c<br>
> +++ b/tests/queue-test.c<br>
> @@ -27,6 +27,7 @@<br>
> #include <sys/types.h><br>
> #include <sys/wait.h><br>
> #include <assert.h><br>
> +#include <pthread.h><br>
><br>
> #include "wayland-client.h"<br>
> #include "wayland-server.h"<br>
> @@ -139,6 +140,53 @@ client_test_multiple_queues(void)<br>
> exit(ret == -1 ? -1 : 0);<br>
> }<br>
><br>
> +/* test that events will come to the right queue */<br>
> +static void<br>
> +client_test_multiple_queues2(void)<br>
> +{<br>
> + struct wl_event_queue *queue[5], *q;<br>
<br>
</div></div>Instead of using 5 everwhere, how about a #define?<br></blockquote><div><br></div><div>Yes, that'll be better.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class=""><br>
> + struct wl_callback *callback;<br>
> + struct wl_display *display;<br>
> + int i, j;<br>
> +<br>
> + display = wl_display_connect(NULL);<br>
> + assert(display);<br>
> +<br>
> + for (i = 0; i < 5; ++i) {<br>
> + queue[i] = wl_display_create_queue(display);<br>
> + assert(queue[i]);<br>
> + }<br>
> +<br>
> + for (i = 0; i < 100; ++i) {<br>
> + q = queue[i % 5];<br>
> + callback = wl_display_sync(display);<br>
<br>
</div>Isn't this 'callback' leaked?<br>
<div class=""><br>
> + assert(callback != NULL);<br>
> + wl_proxy_set_queue((struct wl_proxy *) callback, q);<br>
> +<br>
> + assert(wl_display_flush(display) > 0);<br>
> +<br>
> + /* the callback should come to the q queue */<br>
> + assert(wl_display_dispatch_pending(display) == 0);<br>
> + assert(wl_display_dispatch_queue(display, q) > 0);<br>
> + /* now all queues should be empty */<br>
> + for (j = 0; j < 5; ++j)<br>
> + assert(wl_display_dispatch_queue_pending(display,<br>
> + queue[j]) == 0);<br>
> + }<br>
> +<br>
> + callback = wl_display_sync(display);<br>
<br>
</div>And this?<br></blockquote><div><br></div><div>I'll add a listener that will destroy the callbacks.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div> <br></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div> </div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5">
> + assert(callback != NULL);<br>
> +<br>
> + assert(wl_display_flush(display) > 0);<br>
> + assert(wl_display_dispatch(display) > 0);<br>
> + for (j = 0; j < 5; ++j) {<br>
> + assert(wl_display_dispatch_queue_pending(display, queue[j]) == 0);<br>
> + wl_event_queue_destroy(queue[j]);<br>
> + }<br>
> +<br>
> + wl_display_disconnect(display);<br>
> +}<br>
> +<br>
> static void<br>
> dummy_bind(struct wl_client *client,<br>
> void *data, uint32_t version, uint32_t id)<br>
> @@ -169,5 +217,85 @@ TEST(queue)<br>
> client_create(d, client_test_multiple_queues);<br>
> display_run(d);<br>
><br>
> + client_create(d, client_test_multiple_queues2);<br>
> + display_run(d);<br>
> +<br>
> + display_destroy(d);<br>
> +}<br>
> +<br>
> +struct thread_data {<br>
> + struct wl_display *display;<br>
> + struct wl_event_queue *queue;<br>
> +};<br>
> +<br>
> +static void *<br>
> +run_new_queue(void *arg)<br>
> +{<br>
> + struct thread_data *data = arg;<br>
> + int ret;<br>
> +<br>
> + /* XXX shouldn't it return -1 in the first dispatch?<br>
> + * Anyway - the queue keeps polling atm and is not interrupted by<br>
> + * the error */<br>
<br>
</div></div>Hmm, yes, I think would probably be good that if wl_display goes into<br>
error state, all the dispatch calls should return with error, rather<br>
than hang.<br>
<div><div class="h5"><br></div></div></blockquote><div><br></div><div>Actually, this test-case is wrong. When I was writing this, I did not know anything about threading in Wayland.<br></div><div>Nowadays, I know that this is exactly the case where wl_display_prepare_read + wl_display_read_events should<br>
be used (more threads are reading from the display's fd), so when programmer will do what is in this test,<br>it is his fault. However, I think I could send a path that clarifies this in the documentation<br></div><div>
(I've already sent one, but that is only for wl_display_prepare_read and there's no note about not using wl_display_dispatch concurrently: <a href="http://lists.freedesktop.org/archives/wayland-devel/2014-August/016296.html">http://lists.freedesktop.org/archives/wayland-devel/2014-August/016296.html</a>)<br>
<br></div><div>However, this test-case shows that it is possible to have one thread polling and the other successfully dispatching events<br></div><div>without interrupting the poll, so I'll try to find out if it can happen in some correct way.<br>
</div><div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">
> + do {<br>
> + ret = wl_display_dispatch_queue(data->display, data->queue);<br>
> + /* it definitely cannot dispatch any event */<br>
> + assert(ret <= 0);<br>
> + } while (ret != -1);<br>
> +<br>
> + pthread_exit(NULL);<br>
> +}<br>
> +<br>
> +static void<br>
> +client_test_multiple_queues_errors(void)<br>
> +{<br>
> + struct client *c;<br>
> + struct wl_event_queue *queue;<br>
> + struct thread_data data;<br>
> + pthread_t thr;<br>
> +<br>
> + c = client_connect();<br>
> +<br>
> + queue = wl_display_create_queue(c->wl_display);<br>
> + assert(queue);<br>
> +<br>
> + data.display = c->wl_display;<br>
> + data.queue = queue;<br>
> + /* run new thread that will wait for event in<br>
> + * the new queue. Then post an error and check<br>
> + * if the dispatching was cancelled... */<br>
> + assert(pthread_create(&thr, NULL, run_new_queue, &data) == 0);<br>
> +<br>
> + stop_display(c, 1);<br>
> +<br>
> + /* make sure we got the error */<br>
> + alarm(3); /* timeout */<br>
> + wl_display_roundtrip(c->wl_display);<br>
> + assert(wl_display_get_error(c->wl_display) != 0);<br>
> +<br>
> + /* wait for the thread to finish */<br>
> + assert(pthread_join(thr, NULL) == 0);<br>
> +<br>
> + /* do not use client_disconnect(c), because it would<br>
> + * fail the test since we posted an error */<br>
> + wl_event_queue_destroy(queue);<br>
> + wl_proxy_destroy((struct wl_proxy *) c->tc);<br>
> + wl_display_disconnect(c->wl_display);<br>
> +}<br>
> +<br>
> +TEST(queue_errors)<br>
> +{<br>
> + struct display *d = display_create();<br>
> +<br>
> + client_create(d, client_test_multiple_queues_errors);<br>
> + display_run(d);<br>
> +<br>
> + /* get the client */<br>
> + struct client_info *ci = wl_container_of(d->clients.next, ci, link);<br>
> + assert(ci && ci->wl_client != NULL);<br>
> +<br>
> + wl_client_post_no_memory(ci->wl_client);<br>
> + display_resume(d);<br>
> +<br>
> display_destroy(d);<br>
> }<br>
<br>
<br>
</div></div>Very nice work! :-)<br>
<br>
<br>
Thanks,<br>
pq</blockquote><div><br></div><div>Regards,<br>Marek <br></div></div></div></div>