client: update documentation about threading

Submitted by Marek Chalupa on Nov. 21, 2014, 10:12 a.m.

Details

Message ID 1416564755-6856-1-git-send-email-mchqwerty@gmail.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Marek Chalupa Nov. 21, 2014, 10:12 a.m.
Remove out-dated documentation and add few more words
about this topic.

Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
---
 src/wayland-client.c | 106 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 87 insertions(+), 19 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/wayland-client.c b/src/wayland-client.c
index 41c49e9..b1a1fa0 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -909,6 +909,12 @@  static const struct wl_callback_listener sync_listener = {
  * Blocks until the server process all currently issued requests and
  * sends out pending events on the event queue.
  *
+ * \note This function uses wl_display_dispatch_queue() internally. If you
+ * are using wl_display_read_events() from more threads, don't use this function
+ * (or make sure that calling wl_display_roundtrip_queue() doesn't interfere
+ * with calling wl_display_prepare_read() and wl_display_read_events())
+ *
+ * \sa wl_display_roundtrip()
  * \memberof wl_display
  */
 WL_EXPORT int
@@ -940,6 +946,11 @@  wl_display_roundtrip_queue(struct wl_display *display, struct wl_event_queue *qu
  * Blocks until the server process all currently issued requests and
  * sends out pending events on the default event queue.
  *
+ * \note This function uses wl_display_dispatch_queue() internally. If you
+ * are using wl_display_read_events() from more threads, don't use this function
+ * (or make sure that calling wl_display_roundtrip() doesn't interfere
+ * with calling wl_display_prepare_read() and wl_display_read_events())
+ *
  * \memberof wl_display
  */
 WL_EXPORT int
@@ -1211,14 +1222,55 @@  cancel_read(struct wl_display *display)
  *
  * This will read events from the file descriptor for the display.
  * This function does not dispatch events, it only reads and queues
- * events into their corresponding event queues.  If no data is
+ * events into their corresponding event queues. If no data is
  * avilable on the file descriptor, wl_display_read_events() returns
- * immediately.  To dispatch events that may have been queued, call
- * wl_display_dispatch_pending() or
- * wl_display_dispatch_queue_pending().
+ * immediately. To dispatch events that may have been queued, call
+ * wl_display_dispatch_pending() or wl_display_dispatch_queue_pending().
  *
  * Before calling this function, wl_display_prepare_read() must be
- * called first.
+ * called first. When running in more threads (which is the usual
+ * case, since we'd use wl_display_dispatch() otherwise), every thread
+ * must call wl_display_prepare_read() before calling this function.
+ *
+ * After calling wl_display_prepare_read() there can be some extra code
+ * before calling wl_display_read_events(), for example poll() or alike.
+ * Example of code in a thread:
+ *
+ * \code
+ *
+ *   while (wl_display_prepare_read(display) < 0)
+ *           wl_display_dispatch_pending(display);
+ *   wl_display_flush(display);
+ *
+ *   ... some code ...
+ *
+ *   fds[0].fd = wl_display_get_fd(display);
+ *   fds[0].event = POLLIN | POLLHUP | POLLERR;
+ *   poll(fds, 1, -1);
+ *
+ *   if (!everything_ok()) {
+ *	wl_display_cancel_read(display);
+ *	handle_error();
+ *   }
+ *
+ *   if (wl_display_read_events(display) < 0)
+ *	handle_error();
+ *
+ *   ...
+ * \endcode
+ *
+ * The code should be as short as possible since other threads may
+ * get sleeping until all threads called wl_display_read_events()
+ * or wl_display_cancel_read().
+ *
+ * This function must not be called simultaneously with wl_display_dispatch().
+ * It may lead to deadlock. If programmer wants, for some reason, use
+ * wl_display_dispatch() in one thread and wl_display_read_events() in another,
+ * extra care must be taken to serialize these calls, i. e. use mutexes or
+ * similar.
+ *
+ * \sa wl_display_prepare_read(), wl_display_cancel_read(),
+ * wl_display_dispatch_pending(), wl_display_dispatch()
  *
  * \memberof wl_display
  */
@@ -1296,17 +1348,17 @@  wl_display_prepare_read_queue(struct wl_display *display,
 	return ret;
 }
 
-/** Prepare to read events after polling file descriptor
+/** Prepare to read events from the display's file descriptor
  *
  * \param display The display context object
  * \return 0 on success or -1 if event queue was not empty
  *
  * This function must be called before reading from the file
- * descriptor using wl_display_read_events().  Calling
- * wl_display_prepare_read() announces the calling threads intention
+ * descriptor using wl_display_read_events(). Calling
+ * wl_display_prepare_read() announces the calling thread's intention
  * to read and ensures that until the thread is ready to read and
  * calls wl_display_read_events(), no other thread will read from the
- * file descriptor.  This only succeeds if the event queue is empty
+ * file descriptor. This only succeeds if the event queue is empty
  * though, and if there are undispatched events in the queue, -1 is
  * returned and errno set to EAGAIN.
  *
@@ -1314,6 +1366,13 @@  wl_display_prepare_read_queue(struct wl_display *display,
  * either call wl_display_read_events() when it's ready or cancel the
  * read intention by calling wl_display_cancel_read().
  *
+ * This function doesn't acquire exclusive access to the display's fd.
+ * It only registers that the thread calling this function has intention to
+ * read from fd. When all registered readers call wl_display_read_events(),
+ * only one (at random) eventually reads and queues the events and the
+ * others are sleeping meanwhile. This way we avoid races and still
+ * can read from more threads.
+ *
  * Use this function before polling on the display fd or to integrate
  * the fd into a toolkit event loop in a race-free way.  Typically, a
  * toolkit will call wl_display_dispatch_pending() before sleeping, to
@@ -1350,9 +1409,10 @@  wl_display_prepare_read_queue(struct wl_display *display,
  * Here we call wl_display_prepare_read(), which ensures that between
  * returning from that call and eventually calling
  * wl_display_read_events(), no other thread will read from the fd and
- * queue events in our queue.  If the call to
- * wl_display_prepare_read() fails, we dispatch the pending events and
- * try again until we're successful.
+ * queue events in our queue. If the call to wl_display_prepare_read() fails,
+ * we dispatch the pending events and try again until we're successful.
+ *
+ * \sa wl_display_cancel_read(), wl_display_read_events()
  *
  * \memberof wl_display
  */
@@ -1362,13 +1422,15 @@  wl_display_prepare_read(struct wl_display *display)
 	return wl_display_prepare_read_queue(display, &display->default_queue);
 }
 
-/** Release exclusive access to display file descriptor
+/** Cancel read intention on display's fd
  *
  * \param display The display context object
  *
- * This releases the exclusive access.  Useful for canceling the lock
- * when a timed out poll returns fd not readable and we're not going
- * to read from the fd anytime soon.
+ * After a thread successfully called wl_display_preapare_read() it must
+ * either call wl_display_read_events() or wl_display_cancel_read().
+ * If the threads do not follow this rule it will lead to deadlock.
+ *
+ * \sa wl_display_prepare_read(), wl_display_read_events()
  *
  * \memberof wl_display
  */
@@ -1396,6 +1458,9 @@  wl_display_cancel_read(struct wl_display *display)
  * threads this will block until the main thread queues events on the queue
  * passed as argument.
  *
+ * For behaviour in multi-threaded environment see wl_display_read_events()
+ * and wl_display_prepare_read()
+ *
  * \memberof wl_display
  */
 WL_EXPORT int
@@ -1502,10 +1567,13 @@  wl_display_dispatch_queue_pending(struct wl_display *display,
  * or not. For dispatching main queue events without blocking, see \ref
  * wl_display_dispatch_pending().
  *
- * \note Calling this will release the display file descriptor if this
- * thread acquired it using wl_display_acquire_fd().
+ * In multi-threaded environment, programmer may want to use
+ * wl_display_read_events(). However, use of wl_display_read_events()
+ * must not be mixed with wl_display_dispatch(). See wl_display_read_events()
+ * and wl_display_prepare_read() for more details.
  *
- * \sa wl_display_dispatch_pending(), wl_display_dispatch_queue()
+ * \sa wl_display_dispatch_pending(), wl_display_dispatch_queue(),
+ * wl_display_read_events()
  *
  * \memberof wl_display
  */

Comments

On Fri, 21 Nov 2014 11:12:35 +0100
Marek Chalupa <mchqwerty@gmail.com> wrote:

> Remove out-dated documentation and add few more words
> about this topic.
> 
> Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
> ---
>  src/wayland-client.c | 106 ++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 87 insertions(+), 19 deletions(-)
> 
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 41c49e9..b1a1fa0 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -909,6 +909,12 @@ static const struct wl_callback_listener sync_listener = {
>   * Blocks until the server process all currently issued requests and
>   * sends out pending events on the event queue.
>   *
> + * \note This function uses wl_display_dispatch_queue() internally. If you
> + * are using wl_display_read_events() from more threads, don't use this function
> + * (or make sure that calling wl_display_roundtrip_queue() doesn't interfere
> + * with calling wl_display_prepare_read() and wl_display_read_events())
> + *
> + * \sa wl_display_roundtrip()
>   * \memberof wl_display
>   */
>  WL_EXPORT int
> @@ -940,6 +946,11 @@ wl_display_roundtrip_queue(struct wl_display *display, struct wl_event_queue *qu
>   * Blocks until the server process all currently issued requests and
>   * sends out pending events on the default event queue.
>   *
> + * \note This function uses wl_display_dispatch_queue() internally. If you
> + * are using wl_display_read_events() from more threads, don't use this function
> + * (or make sure that calling wl_display_roundtrip() doesn't interfere
> + * with calling wl_display_prepare_read() and wl_display_read_events())
> + *
>   * \memberof wl_display
>   */
>  WL_EXPORT int
> @@ -1211,14 +1222,55 @@ cancel_read(struct wl_display *display)
>   *
>   * This will read events from the file descriptor for the display.
>   * This function does not dispatch events, it only reads and queues
> - * events into their corresponding event queues.  If no data is
> + * events into their corresponding event queues. If no data is
>   * avilable on the file descriptor, wl_display_read_events() returns
> - * immediately.  To dispatch events that may have been queued, call
> - * wl_display_dispatch_pending() or
> - * wl_display_dispatch_queue_pending().
> + * immediately. To dispatch events that may have been queued, call
> + * wl_display_dispatch_pending() or wl_display_dispatch_queue_pending().
>   *
>   * Before calling this function, wl_display_prepare_read() must be
> - * called first.
> + * called first. When running in more threads (which is the usual
> + * case, since we'd use wl_display_dispatch() otherwise), every thread
> + * must call wl_display_prepare_read() before calling this function.
> + *
> + * After calling wl_display_prepare_read() there can be some extra code
> + * before calling wl_display_read_events(), for example poll() or alike.
> + * Example of code in a thread:
> + *
> + * \code
> + *
> + *   while (wl_display_prepare_read(display) < 0)
> + *           wl_display_dispatch_pending(display);
> + *   wl_display_flush(display);
> + *
> + *   ... some code ...
> + *
> + *   fds[0].fd = wl_display_get_fd(display);
> + *   fds[0].event = POLLIN | POLLHUP | POLLERR;
> + *   poll(fds, 1, -1);
> + *
> + *   if (!everything_ok()) {
> + *	wl_display_cancel_read(display);
> + *	handle_error();
> + *   }
> + *
> + *   if (wl_display_read_events(display) < 0)
> + *	handle_error();
> + *
> + *   ...
> + * \endcode
> + *
> + * The code should be as short as possible since other threads may
> + * get sleeping until all threads called wl_display_read_events()
> + * or wl_display_cancel_read().

Hi,

this paragraph is a bit confusing, "short code" is not a clear term.
There is also the poll() which may take an arbitary amount of time to
return. I probably see what you are getting at, but it should be
rephrased.

How about something like:

After wl_display_prepare_read() succeeds, other threads that enter
wl_display_read_events() will sleep until the very last thread enters
it too or cancels. Therefore when the display fd becomes (or already
is) readable, wl_display_read_events() should be called as soon as
possible to unblock all threads. If wl_display_read_events() will not
be called, then wl_display_cancel_read() must be called
posthaste instead to let the other threads continue.

> + *
> + * This function must not be called simultaneously with wl_display_dispatch().
> + * It may lead to deadlock. If programmer wants, for some reason, use
> + * wl_display_dispatch() in one thread and wl_display_read_events() in another,
> + * extra care must be taken to serialize these calls, i. e. use mutexes or
> + * similar.

Hmm, that mutex should protect not only wl_display_read_events() call,
but the whole sequence between prepare_read and
read_events/cancel_read, should it not?

> + *
> + * \sa wl_display_prepare_read(), wl_display_cancel_read(),
> + * wl_display_dispatch_pending(), wl_display_dispatch()
>   *
>   * \memberof wl_display
>   */
> @@ -1296,17 +1348,17 @@ wl_display_prepare_read_queue(struct wl_display *display,
>  	return ret;
>  }
>  
> -/** Prepare to read events after polling file descriptor
> +/** Prepare to read events from the display's file descriptor
>   *
>   * \param display The display context object
>   * \return 0 on success or -1 if event queue was not empty
>   *
>   * This function must be called before reading from the file
> - * descriptor using wl_display_read_events().  Calling
> - * wl_display_prepare_read() announces the calling threads intention
> + * descriptor using wl_display_read_events(). Calling
> + * wl_display_prepare_read() announces the calling thread's intention
>   * to read and ensures that until the thread is ready to read and
>   * calls wl_display_read_events(), no other thread will read from the
> - * file descriptor.  This only succeeds if the event queue is empty
> + * file descriptor. This only succeeds if the event queue is empty
>   * though, and if there are undispatched events in the queue, -1 is
>   * returned and errno set to EAGAIN.
>   *
> @@ -1314,6 +1366,13 @@ wl_display_prepare_read_queue(struct wl_display *display,
>   * either call wl_display_read_events() when it's ready or cancel the
>   * read intention by calling wl_display_cancel_read().
>   *
> + * This function doesn't acquire exclusive access to the display's fd.
> + * It only registers that the thread calling this function has intention to
> + * read from fd. When all registered readers call wl_display_read_events(),
> + * only one (at random) eventually reads and queues the events and the
> + * others are sleeping meanwhile. This way we avoid races and still
> + * can read from more threads.
> + *
>   * Use this function before polling on the display fd or to integrate
>   * the fd into a toolkit event loop in a race-free way.  Typically, a
>   * toolkit will call wl_display_dispatch_pending() before sleeping, to
> @@ -1350,9 +1409,10 @@ wl_display_prepare_read_queue(struct wl_display *display,
>   * Here we call wl_display_prepare_read(), which ensures that between
>   * returning from that call and eventually calling
>   * wl_display_read_events(), no other thread will read from the fd and
> - * queue events in our queue.  If the call to
> - * wl_display_prepare_read() fails, we dispatch the pending events and
> - * try again until we're successful.
> + * queue events in our queue. If the call to wl_display_prepare_read() fails,
> + * we dispatch the pending events and try again until we're successful.
> + *
> + * \sa wl_display_cancel_read(), wl_display_read_events()
>   *
>   * \memberof wl_display
>   */
> @@ -1362,13 +1422,15 @@ wl_display_prepare_read(struct wl_display *display)
>  	return wl_display_prepare_read_queue(display, &display->default_queue);
>  }
>  
> -/** Release exclusive access to display file descriptor
> +/** Cancel read intention on display's fd
>   *
>   * \param display The display context object
>   *
> - * This releases the exclusive access.  Useful for canceling the lock
> - * when a timed out poll returns fd not readable and we're not going
> - * to read from the fd anytime soon.
> + * After a thread successfully called wl_display_preapare_read() it must

Typo: preapare.

> + * either call wl_display_read_events() or wl_display_cancel_read().
> + * If the threads do not follow this rule it will lead to deadlock.
> + *
> + * \sa wl_display_prepare_read(), wl_display_read_events()
>   *
>   * \memberof wl_display
>   */
> @@ -1396,6 +1458,9 @@ wl_display_cancel_read(struct wl_display *display)
>   * threads this will block until the main thread queues events on the queue
>   * passed as argument.
>   *
> + * For behaviour in multi-threaded environment see wl_display_read_events()
> + * and wl_display_prepare_read()

Shouldn't we have the same warning here for wl_display_dispatch_queue()
as we have below for wl_display_dispatch()?

Only the *_pending() function are safe in a multithreaded app, right?


> + *
>   * \memberof wl_display
>   */
>  WL_EXPORT int
> @@ -1502,10 +1567,13 @@ wl_display_dispatch_queue_pending(struct wl_display *display,
>   * or not. For dispatching main queue events without blocking, see \ref
>   * wl_display_dispatch_pending().
>   *
> - * \note Calling this will release the display file descriptor if this
> - * thread acquired it using wl_display_acquire_fd().
> + * In multi-threaded environment, programmer may want to use
> + * wl_display_read_events(). However, use of wl_display_read_events()
> + * must not be mixed with wl_display_dispatch(). See wl_display_read_events()
> + * and wl_display_prepare_read() for more details.
>   *
> - * \sa wl_display_dispatch_pending(), wl_display_dispatch_queue()
> + * \sa wl_display_dispatch_pending(), wl_display_dispatch_queue(),
> + * wl_display_read_events()
>   *
>   * \memberof wl_display
>   */

Meanwhile I have merged some patches that make this one need a rebase.

Otherwise it looks ok to me.


Thanks,
pq
On 26 November 2014 at 13:48, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 21 Nov 2014 11:12:35 +0100
> Marek Chalupa <mchqwerty@gmail.com> wrote:
>
> > Remove out-dated documentation and add few more words
> > about this topic.
> >
> > Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
> > ---
> >  src/wayland-client.c | 106
> ++++++++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 87 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/wayland-client.c b/src/wayland-client.c
> > index 41c49e9..b1a1fa0 100644
> > --- a/src/wayland-client.c
> > +++ b/src/wayland-client.c
> > @@ -909,6 +909,12 @@ static const struct wl_callback_listener
> sync_listener = {
> >   * Blocks until the server process all currently issued requests and
> >   * sends out pending events on the event queue.
> >   *
> > + * \note This function uses wl_display_dispatch_queue() internally. If
> you
> > + * are using wl_display_read_events() from more threads, don't use this
> function
> > + * (or make sure that calling wl_display_roundtrip_queue() doesn't
> interfere
> > + * with calling wl_display_prepare_read() and wl_display_read_events())
> > + *
> > + * \sa wl_display_roundtrip()
> >   * \memberof wl_display
> >   */
> >  WL_EXPORT int
> > @@ -940,6 +946,11 @@ wl_display_roundtrip_queue(struct wl_display
> *display, struct wl_event_queue *qu
> >   * Blocks until the server process all currently issued requests and
> >   * sends out pending events on the default event queue.
> >   *
> > + * \note This function uses wl_display_dispatch_queue() internally. If
> you
> > + * are using wl_display_read_events() from more threads, don't use this
> function
> > + * (or make sure that calling wl_display_roundtrip() doesn't interfere
> > + * with calling wl_display_prepare_read() and wl_display_read_events())
> > + *
> >   * \memberof wl_display
> >   */
> >  WL_EXPORT int
> > @@ -1211,14 +1222,55 @@ cancel_read(struct wl_display *display)
> >   *
> >   * This will read events from the file descriptor for the display.
> >   * This function does not dispatch events, it only reads and queues
> > - * events into their corresponding event queues.  If no data is
> > + * events into their corresponding event queues. If no data is
> >   * avilable on the file descriptor, wl_display_read_events() returns
> > - * immediately.  To dispatch events that may have been queued, call
> > - * wl_display_dispatch_pending() or
> > - * wl_display_dispatch_queue_pending().
> > + * immediately. To dispatch events that may have been queued, call
> > + * wl_display_dispatch_pending() or wl_display_dispatch_queue_pending().
> >   *
> >   * Before calling this function, wl_display_prepare_read() must be
> > - * called first.
> > + * called first. When running in more threads (which is the usual
> > + * case, since we'd use wl_display_dispatch() otherwise), every thread
> > + * must call wl_display_prepare_read() before calling this function.
> > + *
> > + * After calling wl_display_prepare_read() there can be some extra code
> > + * before calling wl_display_read_events(), for example poll() or alike.
> > + * Example of code in a thread:
> > + *
> > + * \code
> > + *
> > + *   while (wl_display_prepare_read(display) < 0)
> > + *           wl_display_dispatch_pending(display);
> > + *   wl_display_flush(display);
> > + *
> > + *   ... some code ...
> > + *
> > + *   fds[0].fd = wl_display_get_fd(display);
> > + *   fds[0].event = POLLIN | POLLHUP | POLLERR;
> > + *   poll(fds, 1, -1);
> > + *
> > + *   if (!everything_ok()) {
> > + *   wl_display_cancel_read(display);
> > + *   handle_error();
> > + *   }
> > + *
> > + *   if (wl_display_read_events(display) < 0)
> > + *   handle_error();
> > + *
> > + *   ...
> > + * \endcode
> > + *
> > + * The code should be as short as possible since other threads may
> > + * get sleeping until all threads called wl_display_read_events()
> > + * or wl_display_cancel_read().
>
> Hi,
>
> this paragraph is a bit confusing, "short code" is not a clear term.
> There is also the poll() which may take an arbitary amount of time to
> return. I probably see what you are getting at, but it should be
> rephrased.
>
> How about something like:
>
> After wl_display_prepare_read() succeeds, other threads that enter
> wl_display_read_events() will sleep until the very last thread enters
> it too or cancels. Therefore when the display fd becomes (or already
> is) readable, wl_display_read_events() should be called as soon as
> possible to unblock all threads. If wl_display_read_events() will not
> be called, then wl_display_cancel_read() must be called
> posthaste instead to let the other threads continue.
>

Yes, that's clearer :)


>
> > + *
> > + * This function must not be called simultaneously with
> wl_display_dispatch().
> > + * It may lead to deadlock. If programmer wants, for some reason, use
> > + * wl_display_dispatch() in one thread and wl_display_read_events() in
> another,
> > + * extra care must be taken to serialize these calls, i. e. use mutexes
> or
> > + * similar.
>
> Hmm, that mutex should protect not only wl_display_read_events() call,
> but the whole sequence between prepare_read and
> read_events/cancel_read, should it not?
>

Yup


>
> > + *
> > + * \sa wl_display_prepare_read(), wl_display_cancel_read(),
> > + * wl_display_dispatch_pending(), wl_display_dispatch()
> >   *
> >   * \memberof wl_display
> >   */
> > @@ -1296,17 +1348,17 @@ wl_display_prepare_read_queue(struct wl_display
> *display,
> >       return ret;
> >  }
> >
> > -/** Prepare to read events after polling file descriptor
> > +/** Prepare to read events from the display's file descriptor
> >   *
> >   * \param display The display context object
> >   * \return 0 on success or -1 if event queue was not empty
> >   *
> >   * This function must be called before reading from the file
> > - * descriptor using wl_display_read_events().  Calling
> > - * wl_display_prepare_read() announces the calling threads intention
> > + * descriptor using wl_display_read_events(). Calling
> > + * wl_display_prepare_read() announces the calling thread's intention
> >   * to read and ensures that until the thread is ready to read and
> >   * calls wl_display_read_events(), no other thread will read from the
> > - * file descriptor.  This only succeeds if the event queue is empty
> > + * file descriptor. This only succeeds if the event queue is empty
> >   * though, and if there are undispatched events in the queue, -1 is
> >   * returned and errno set to EAGAIN.
> >   *
> > @@ -1314,6 +1366,13 @@ wl_display_prepare_read_queue(struct wl_display
> *display,
> >   * either call wl_display_read_events() when it's ready or cancel the
> >   * read intention by calling wl_display_cancel_read().
> >   *
> > + * This function doesn't acquire exclusive access to the display's fd.
> > + * It only registers that the thread calling this function has
> intention to
> > + * read from fd. When all registered readers call
> wl_display_read_events(),
> > + * only one (at random) eventually reads and queues the events and the
> > + * others are sleeping meanwhile. This way we avoid races and still
> > + * can read from more threads.
> > + *
> >   * Use this function before polling on the display fd or to integrate
> >   * the fd into a toolkit event loop in a race-free way.  Typically, a
> >   * toolkit will call wl_display_dispatch_pending() before sleeping, to
> > @@ -1350,9 +1409,10 @@ wl_display_prepare_read_queue(struct wl_display
> *display,
> >   * Here we call wl_display_prepare_read(), which ensures that between
> >   * returning from that call and eventually calling
> >   * wl_display_read_events(), no other thread will read from the fd and
> > - * queue events in our queue.  If the call to
> > - * wl_display_prepare_read() fails, we dispatch the pending events and
> > - * try again until we're successful.
> > + * queue events in our queue. If the call to wl_display_prepare_read()
> fails,
> > + * we dispatch the pending events and try again until we're successful.
> > + *
> > + * \sa wl_display_cancel_read(), wl_display_read_events()
> >   *
> >   * \memberof wl_display
> >   */
> > @@ -1362,13 +1422,15 @@ wl_display_prepare_read(struct wl_display
> *display)
> >       return wl_display_prepare_read_queue(display,
> &display->default_queue);
> >  }
> >
> > -/** Release exclusive access to display file descriptor
> > +/** Cancel read intention on display's fd
> >   *
> >   * \param display The display context object
> >   *
> > - * This releases the exclusive access.  Useful for canceling the lock
> > - * when a timed out poll returns fd not readable and we're not going
> > - * to read from the fd anytime soon.
> > + * After a thread successfully called wl_display_preapare_read() it must
>
> Typo: preapare.
>
> > + * either call wl_display_read_events() or wl_display_cancel_read().
> > + * If the threads do not follow this rule it will lead to deadlock.
> > + *
> > + * \sa wl_display_prepare_read(), wl_display_read_events()
> >   *
> >   * \memberof wl_display
> >   */
> > @@ -1396,6 +1458,9 @@ wl_display_cancel_read(struct wl_display *display)
> >   * threads this will block until the main thread queues events on the
> queue
> >   * passed as argument.
> >   *
> > + * For behaviour in multi-threaded environment see
> wl_display_read_events()
> > + * and wl_display_prepare_read()
>
> Shouldn't we have the same warning here for wl_display_dispatch_queue()
> as we have below for wl_display_dispatch()?
>
> Only the *_pending() function are safe in a multithreaded app, right?
>
>
Yes, only the ones that do not attempt to read from the fd.


>
> > + *
> >   * \memberof wl_display
> >   */
> >  WL_EXPORT int
> > @@ -1502,10 +1567,13 @@ wl_display_dispatch_queue_pending(struct
> wl_display *display,
> >   * or not. For dispatching main queue events without blocking, see \ref
> >   * wl_display_dispatch_pending().
> >   *
> > - * \note Calling this will release the display file descriptor if this
> > - * thread acquired it using wl_display_acquire_fd().
> > + * In multi-threaded environment, programmer may want to use
> > + * wl_display_read_events(). However, use of wl_display_read_events()
> > + * must not be mixed with wl_display_dispatch(). See
> wl_display_read_events()
> > + * and wl_display_prepare_read() for more details.
> >   *
> > - * \sa wl_display_dispatch_pending(), wl_display_dispatch_queue()
> > + * \sa wl_display_dispatch_pending(), wl_display_dispatch_queue(),
> > + * wl_display_read_events()
> >   *
> >   * \memberof wl_display
> >   */
>
> Meanwhile I have merged some patches that make this one need a rebase.
>
> Otherwise it looks ok to me.
>
>
> Thanks,
> pq
>


Thanks,
Marek