Thread affinity

[Andreas Ericsson]

On 03/05/2012 01:14 PM, Jørgen Lind wrote:
> Hi,
> 
> I had some time today update the patch to add thread affinity to
> wayland-clients wl_display.
> 

Not that anyone else seem to have picked up this patch, but I've got
some small issues with it even so.

Code is copy-pasted back in (why attach?), so it will probably look
weird and the reply-to markers are missing.

@@ -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));




@@ -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.

--  
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.