Thread affinity

[Jørgen Lind]

Thanks for taking your time to look at the patch :)

> Code is copy-pasted back in (why attach?), so it will probably look
> weird and the reply-to markers are missing.
Looks fine to me.
> @@ -212,6 +220,14 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
>  		wl_closure_print(closure, &proxy->object, true);
>  
>  	wl_closure_destroy(closure);
> +
> +	write_notification_value = 1;
> +	success = write(proxy->display->write_notification_event_fd,&write_notification_value,8);
> +	if (success < 0) {
> +		fprintf(stderr, "Error writing to eventfd\n");
> +	}
> 
> Please make a habit to always include the strerror(errno) string for
> failed system calls. It makes it a lot more pleasant to debug. In this
> case, I would probably word it
> 
> fprintf(stderr, "Error writing to eventfd %d: %s\n",
> 	proxy->display->write_notification_event_fd,
> 	strerror(errno));

Yes your right.

> 
> @@ -396,6 +413,21 @@ wl_display_connect(const char *name)
>  		return NULL;
>  	}
>  
> +	display->write_notification_event_fd = eventfd(0, EFD_CLOEXEC);
> +        if (display->write_notification_event_fd < 0) {
> +            fprintf(stderr, "Failed to create eventfd\n");
> +        }
> 
> strerror(errno) should be here too.
> 
> 
> @@ -500,7 +572,17 @@ WL_EXPORT void
>  wl_display_iterate(struct wl_display *display, uint32_t mask)
>  {
>  	uint32_t p[2], object, opcode, size;
> +	uint64_t write_notification_value;
>  	int len;
> +	ssize_t success;
> +
> +	if (pthread_self() != display->thread_id)
> +		fprintf(stderr,
> +			"wl_display_iterate called from another thread than "
> +			"the thead that created wl_display. This will result "
> +			"in undefined behavior\n");
> +
> +	pthread_mutex_lock(&display->marshalling_mutex);
> 
> 
> This makes no sense. If we know we're likely to crash if the thread
> we're being called as isn't the same as the one that created the
> display instance, we should disallow it and return an error instead
> of trying to obtain the lock and proceeding. There's no (sane)
> programmer in the world who *wants* undefined behaviour.

Yeah, undefined behavior is a bitch. Problem is that I was slightly lazy
when I wrote this, and so I called something undefined behavior, which
it actually is not. What happens is that event callbacks will be
delivered in other threads then the on that created the wl_display. This
can be handled correctly like we do in Qt, but it also might lead to
some funny race conditions. With the patch these race conditions will
lead to a deadlock, but will give pretty odd behavior without this
patch.

I don't expect this patch to be picked up, so I added a define so that
the two added functions can safly be used with client code and
fixed error messages.

Jørgen