On Thu, 28 Aug 2014 15:59:37 +0200 Marek Chalupa <mchqwe...@gmail.com> wrote:
> On 28 August 2014 12:02, Pekka Paalanen <ppaala...@gmail.com> wrote: > > > On Mon, 4 Aug 2014 11:30:46 +0200 > > Marek Chalupa <mchqwe...@gmail.com> wrote: > > > > > From the doc comment I get the feeling, that after successfull call to > > > wl_display_prepare_read(), the thread gains exclusive access to the fd. > > > That is not true. It only ensures that _one_ of the threads, that called > > > this function, will read from the fd and there will be no race. > > > > > > Here's slice of the commit message that introduces > > > wl_display_prepare_read() (3c7e8bfbb4745315b7bcbf69fa746c3d6718c305): > > > > > > "wl_display_prepare_read() registers the calling thread as a potential > > > reader of events. Once data is available on the fd, all reader > > > threads must call wl_display_read_events(), at which point > > > one of the threads will read from the fd and distribute the events > > > to event queues. When that is done, all threads return from > > > wl_display_read_events()." > > > > > > That is not clear from the doc message. > > > --- > > > src/wayland-client.c | 27 +++++++++++++++++++++++---- > > > 1 file changed, 23 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/wayland-client.c b/src/wayland-client.c > > > index 19b5694..d7de24d 100644 > > > --- a/src/wayland-client.c > > > +++ b/src/wayland-client.c > > > @@ -1155,6 +1155,12 @@ read_events(struct wl_display *display) > > > pthread_cond_broadcast(&display->reader_cond); > > > } else { > > > serial = display->read_serial; > > > + /* in the case that the thread is not allowed to read > > > + * from the fd, make it sleep until reading and dispatching > > > + * is done. The condition display->read_serial == serial) > > is > > > + * because it may happen, that some thread will call this > > > + * function after the reading is done and without this, the > > > + * thread would block until next pthread_cond_broadcast */ > > > while (display->read_serial == serial) > > > pthread_cond_wait(&display->reader_cond, > > > &display->mutex); > > > > I think the condition here is to ensure, that the function call will > > not return until someone has actually read the fd, not to avoid > > blocking in some cases. Apparently there could be spurious wake-ups, > > and need to protect against them. The 'serial' is set to > > display->read_serial just before waiting, and the mutex is held, so the > > cond_wait is guaranteed to happen at least once. > > > > This branch cannot be entered "after" the reading was done, because > > it is always the last call here that reads, when reader_count hits zero. > > > > > Yes, you're right. That's the real purpose of it. My mistake. However, > AFAIK in the current code this loop > will run only once, because everywhere is the cond broadcast called along > with > increasing the serial. The manual I could find: http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_cond_wait.html says that spurious wakeups may occur. So better keep the serial there. > Regarding the allowed actions.. do you have something particular in mind? > Theoretically, > programmer can safely use everything that doesn't change > display->readers_count. > (basically everything except wl_display_dispatch_queue). > But I don't know why he would do that - the expected action is just to call > wl_display_read_events > (with possibly poll before that) once you're prepared to read. Nothing particular in my mind, I was just wondering if there was any old API that might break this. OTOH, that can be written off as just not using the new API correctly. > > Or better yet, add a chapter to the publican docs about integrating > > libwayland-client to an event loop. I don't recall there being that > > yet... > > > > > > Thanks, > > pq > > > > Thanks for notes, > I'll fix the patch and eventually try to write up some 'big doc' for that. Very good. I think this is one of the areas that are easy to get subtly wrong, so a good doc is valuable. :-) Thanks, pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel