[PATCH] tests: test if thread can block on error

[Pekka Paalanen]

On Tue,  5 Aug 2014 11:39:49 +0200
Marek Chalupa <mchqwerty at gmail.com> wrote:

> wl_display_read_events() can make a thread wait until some other thread
> ends reading. Normally it wakes up all threads after the reading is
> done. But there's a place when it does not get to waking up the threads
> - when an error occurs. This test reveals bug that can block programs.
> 
> If a thread is waiting in wl_display_read_events() and another thread
> calls wl_display_read_events and the reading fails,
> then the sleeping thread is not woken up. This is because
> display_handle_error is using old pthread_cond instead of new
> display->reader_cond, that was added along with wl_display_read_events().
> ---
>  tests/display-test.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 
> diff --git a/tests/display-test.c b/tests/display-test.c
> index e212942..26f946b 100644
> --- a/tests/display-test.c
> +++ b/tests/display-test.c
> @@ -31,6 +31,7 @@
>  #include <errno.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> +#include <pthread.h>
>  
>  #include "wayland-private.h"
>  #include "wayland-server.h"
> @@ -323,3 +324,92 @@ TEST(post_nomem_tst)
>  
>  	display_destroy(d);
>  }
> +
> +static void
> +register_reading(struct wl_display *display)
> +{
> +	while(wl_display_prepare_read(display) != 0 && errno == EAGAIN)
> +		assert(wl_display_dispatch_pending(display) >= 0);
> +	assert(wl_display_flush(display) >= 0);
> +}
> +
> +static void *
> +thread_read_error(void *data)
> +{
> +	struct client *c = data;
> +
> +	register_reading(c->wl_display);
> +
> +	/*
> +	 * Calling the read right now will block this thread
> +	 * until the other thread will read the data.
> +	 * However, after invoking an error, this
> +	 * thread should be woken up or it will block indefinitely.
> +	 */
> +	c->display_stopped = 1;
> +	assert(wl_display_read_events(c->wl_display) == 0);
> +
> +	wl_display_dispatch_pending(c->wl_display);
> +	assert(wl_display_get_error(c->wl_display));
> +
> +	pthread_exit(NULL);
> +}
> +
> +/* test posting an error in multi-threaded environment. */
> +static void
> +threading_post_err(void)
> +{
> +	struct client *c = client_connect();
> +	pthread_t thread;
> +
> +	/* register read intention */
> +	register_reading(c->wl_display);
> +
> +	/* use this var as an indicator that thread is sleeping */
> +	c->display_stopped = 0;
> +
> +	/* create new thread that will register its intention too */
> +	assert(pthread_create(&thread, NULL, thread_read_error, c) == 0);
> +
> +	/* make sure thread is sleeping. It's a little bit racy
> +	 * (setting display_stopped to 1 and calling wl_display_read_events)
> +	 * so call usleep once again after the loop ends - it should
> +	 * be sufficient... */
> +	while (c->display_stopped == 0)
> +		usleep(500);
> +
> +	usleep(10000);
> +
> +	/* so now we have sleeping thread waiting for a pthread_cond signal.
> +	 * The main thread must call wl_display_read_events().
> +	 * If this call fails, then it won't call broadcast at the
> +	 * end of the function and the sleeping thread will block indefinitely.
> +	 * Make the call fail and watch if libwayland will unblock the thread! */
> +
> +	/* create error on fd, so that wl_display_read_events will fail.
> +	 * The same can happen when server hangs up */
> +	close(wl_display_get_fd(c->wl_display));
> +	/* this read events will fail and will
> +	 * post an error that should wake the sleeping thread
> +	 * and dispatch the incoming events */
> +	assert(wl_display_read_events(c->wl_display) == -1);
> +
> +	/* kill test in 3 seconds. This should be enough time for the
> +	 * thread to exit if it's not blocking. If everything is OK, than
> +	 * the thread was woken up and the test will end before the SIGALRM */
> +	alarm(3);
> +	pthread_join(thread, NULL);
> +
> +	wl_proxy_destroy((struct wl_proxy *) c->tc);
> +	wl_display_disconnect(c->wl_display);
> +}
> +
> +TEST(threading_errors_tst)
> +{
> +	struct display *d = display_create();
> +
> +	client_create(d, threading_post_err);
> +	display_run(d);
> +
> +	display_destroy(d);
> +}

This looks good. I'll be merging this and likely the others after I get
the refreshed series adding the test-compositor framework, as discussed
in irc.


Thanks,
pq