<html>
<head>
<base href="https://bugzilla.gnome.org/" />
</head>
<body><span class="vcard"><a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> <span class="fn">Jonas Ådahl</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - gdk/wayland: event source is not multi-thread aware"
href="https://bugzilla.gnome.org/show_bug.cgi?id=763852">bug 763852</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #324334 status</td>
<td>none
</td>
<td>needs-work
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - gdk/wayland: event source is not multi-thread aware"
href="https://bugzilla.gnome.org/show_bug.cgi?id=763852#c9">Comment # 9</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - gdk/wayland: event source is not multi-thread aware"
href="https://bugzilla.gnome.org/show_bug.cgi?id=763852">bug 763852</a>
from <span class="vcard"><a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> <span class="fn">Jonas Ådahl</span></a>
</span></b>
<pre>Review of <span class=""><a href="attachment.cgi?id=324334&action=diff" name="attach_324334" title="gdk/wayland: use the multi-thread safe wayland dispatching API">attachment 324334</a> <a href="attachment.cgi?id=324334&action=edit" title="gdk/wayland: use the multi-thread safe wayland dispatching API">[details]</a></span> <a href='review?bug=763852&attachment=324334'>[review]</a>:
::: gdk/wayland/gdkeventsource.c
@@ +58,3 @@
+ * wl_display_cancel_read() */
+ if (source->reading)
+ wl_display_cancel_read (display->wl_display);
When will this actually happen? Anyway, if it is needed, we should cancel the
read the first thing in this function, so that we don't leave this function
without later polling the fd.
@@ +60,3 @@
+ wl_display_cancel_read (display->wl_display);
+
+ while (wl_display_prepare_read (display->wl_display) != 0)
I think we can just do if (wl_display_prepare_read (display->wl_display) != 0)
return TRUE; as it means we can already dispatch without polling.
We also need to wl_display_prepare_read() on the potential return FALSE; path
above (the first if statement in this function), so that we always have
prepared for reading when we return FALSE.
@@ +81,1 @@
+ if (source->display->event_pause_count > 0) {
nit: { sholud go on its own line.
@@ +90,3 @@
+ {
+ if (wl_display_read_events (display_wayland->wl_display) < 0)
+ g_error ("Error reading events from display: %s", g_strerror (errno));
I think it might be a good idea to either g_assert (source->reading) or
g_return_if_fail (source->reading) before actually reading here.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>