When we release event queue with queued events, we can leak
proxies in some cases.
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
The fact that these functions take both a display and queue argument is
I think historical, and they really are methods on the queue.
Also added some docs for wl_display_prepare_read_queue.
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
Remove out-dated documentation and add few more words
about this topic.
v2. replace a paragraph by better explanation from Pekka Paalanen
fix other notes from reviewing
v3. fix typo
v4. fix flags for poll in an example
add wl_display_cancel_read() to another example
(so that user sees that it should be used)
move proper use of wl_display_prepare_read
before the explanation why it is wrong to use
wl_display_displach
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
1) there is nothing like main thread since
3c7e8bfbb4 anymore, so remove
it from documentation and update the doc accordingly.
2) use calling 'default queue' instead of 'main queue'. In the code
we use display->default_queue, so it'll be easier the understand.
3) update some obsolete or unprecise pieces of documentation
v2. Not only remove out-of-date comment, but fix/remove more
things across the wayland-client.[ch]
v3. fixes (rephrasing unclear paragraphs etc.)
according to Pakka Paalanen notes (thanks)
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
This does not make a difference to doxygen output but may help other
document generators not make redundant links.
Reviewed-by: Bryce Harrington <b.harrington@samsung.com>
(Fixed to remove accidental commit of another change)
After some feedback from Marek Chalupa I decided to just remove this. There
were suggestions about warning about multiple threads but it appears this
would be true for many of these functions and thus it would be misleading to
mention multiple threads only here (as it would imply that multiple threads
work for other functions which is not true, I think).
Acked-by: Marek Chalupa <mchqwerty@gmail.com>
Also removed \comment and used C++ comments. There does not appear
to be any other way to put comments into code samples.
Reviewed-by: Bryce Harrington <b.harrington@samsung.com>
When a thread is sleeping, waiting until another thread read
from the display, it always returns 0. Even when an error
occured. In documentation stands:
"return 0 on success or -1 on error. In case of error errno will
be set accordingly"
So this is a fix for this.
Along with the read_events, fix a test so that it now complies
with this behaviour (and we have this tested)
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Calling wl_display_read_events() after an error should be equivalent
to wl_display_cancel_read(), so that display state is consistent.
Thanks to Pekka Paalanen <pekka.paalanen@collabora.co.uk>
for pointing that out.
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
If wl_connection_read returned EAGAIN, we must wake up sleeping
threads. If we don't do this and the thread calling
wl_connection_read won't call wl_display_read_events again,
the sleeping threads will sleep indefinitely.
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
This helper function wraps the always-repeated pattern:
display->read_serial++;
pthread_cond_broadcast(&display->reader_cond);
[Pekka Paalanen: minor whitespace and comment fixes.]
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Up until now, newly created wl_proxys (with proxy_create or
wl_proxy_create_for_id) are not initialized properly after memory
allocation. The wl_display object in contrast is. To prevent giving
uninitialized data to the user (e.g. user_data) an appropriate memset
has been added. Also, after a memset members don't have to be
explicitly initialized with zero anymore.
Signed-off-by: Nils Chr. Brause <nilschrbrause@googlemail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
This prevents from blocking shown in one display test. Also, it
makes sense to not proceed further in the code of the function
when an error ocurred.
v2. set errno
put note about the errno into wl_display_prepare_read doc
check for error with mutex locked
v3.
set errno to display->last_error
check for the error only in wl_display_read_events. It's sufficient
as prevention for the hanging and programmer doesn't need to
check if wl_display_prepare_read (that was previously covered by
this patch too) returned an error or the queue just was not empty.
Without the check, it could result in indefinite looping.
Thanks to Pekka Paalanen <pekka.paalanen@collabora.co.uk> for
constant reviewing and discussing this patch.
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
In previous commit we removed unused variables. One of them was
pthread_cond_t that was formerly used when reading from display, but
later was (erroneously) made unused. This patch fixes this error
and is a fix for the failing test introduced few patches ago (tests:
test if thread can block on error)
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
The wl_event_queue cond variable has been replaced by the wl_display
reader_cond variable (commit 3c7e8bfbb4).
This cond variable is never waited for anymore, just
signaled/broadcasted, and thus can be safely removed.
The wl_display event_queue_list and link from wl_event_queue
can be removed as well, since it was only used to iterate over
the event queue list in order to broadcast the now unused cond.
No regression on queue unit tests.
Signed-off-by: Olivier Blin <olivier.blin@softathome.com>
v2: fixed and rebased after 886b09c9a3
added signed-off-by
v3: removed link from wl_event_queue
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
display_thread variable is unused since
3c7e8bfbb4
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
wl_display_roundtrip() works on the default queue. Add a parallel
wl_display_roundtrip_queue().
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
When an error occurs, wl_display_get_error() does not
provide any way of getting know if it was a local error or if it was
an error event, respectively what object caused the error and what
the error was.
This patch introduces a new function wl_display_get_protocol_error()
which will return error code, interface and id of the object that
generated the error.
wl_display_get_error() will work the same way as before.
wl_display_get_protocol_error() DOES NOT indicate that a non-protocol
error happened. It returns valid information only in that case that
(protocol) error occurred, so it should be used after calling
wl_display_get_error() with positive result.
[Pekka Paalanen] Applied another hunk of Bryce's comments to docs,
added libtool version bump.
Reviewed-by: Pekka Paalanen <ppaalanen@gmail.com>
Reviewed-by: Bryce Harrington <b.harrington@samsung.com>
errno is supposed to be positive, not negative. It seems that
everything else that calls display_fatal_error() calls it with
a positive error code, so do it here as well.
The wl_display events (error and delete_id) need to be handled even
if the default queue doesn't get dispatched for a while. For example,
a busy EGL rendering loop hits wl_display.sync every eglSwapBuffers()
and we need to process the delete_id events to maintain the object ID
data structure.
As it is, that doesn't happen, but with this change we special case
wl_display events. We put them on a custom, private queue and when
dispatching events, we always dispatch display_queue events first.
The wl_display proxy should still be the default_queue, so that objects
created from wl_display requests get assigned to that.
Restart the poll() if we take a signal. This is easily triggered in
an application that ends up blocking in eglSwapBuffers(), and causes EGL
to fail to allocate a back buffer.
This will be useful in order to implement the
EGL_WL_create_wayland_buffer_from_image extension. The buffers created
within Mesa's Wayland platform are created using the the wl_drm object
as a proxy factory which means they will be set to use Mesa's internal
event queue. However, these buffers will be owned by the client
application so they ideally need to use the default event loop. This
function provides a way to set the proxy's event queue back to the
default.
krh: Edited from Neils original patch to just use wl_proxy_set_queue() with
a NULL argument instead of introducing a new function.
The server requires clients to only allocate one ID ahead of the previously
highest ID in order to keep the ID range tight. Failure to do so will
make the server close the client connection. However, the way we allocate
new IDs is racy. The generated code looks like:
new_proxy = wl_proxy_create(...);
wl_proxy_marshal(proxy, ... new_proxy, ...);
If two threads do this at the same time, there's a chance that thread A
will allocate a proxy, then get pre-empted by thread B which then allocates
a proxy and then passes it to wl_proxy_marshal(). The ID for thread As
proxy will be one higher that the currently highest ID, but the ID for
thread Bs proxy will be two higher. But since thread B prempted thread A
before it could send its new ID, B will send its new ID first, the server
will see the ID from thread Bs proxy first, and will reject it.
We fix this by introducing wl_proxy_marshal_constructor(). This
function is identical to wl_proxy_marshal(), except that it will
allocate a wl_proxy for NEW_ID arguments and send it, all under the
display mutex. By introducing a new function, we maintain backwards
compatibility with older code from the generator, and make sure that
the new generated code has an explicit dependency on a new enough
libwayland-client.so.
A virtual Wayland merit badge goes to Kalle Vahlman, who tracked this
down and analyzed the issue.
Reported-by: Kalle Vahlman <kalle.vahlman@movial.com>
In wl_display_dispatch_queue, if poll fails then it would previously
return immediately and leak a reference in display->reader_count. Then
if the application ignores the error and tries to read again it will
block forever. This can happen for example if the poll fails with
EINTR which the application might consider to be a recoverable error.
This patch makes it cancel the read so the reader_count will be
decremented when poll fails.
This commit adds support for language bindings on the client half of the
library. The idea is the same as for server-side dispatchers.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
This is the mirror function to wl_proxy_add_listener and is useful
inside client libraries to differentiate events on listeners for which
multiple proxies have been created.
We can't do that there, we have to make sure it stays a valid fd until
the application calls wl_display_disconnect(). Otherwise the application
may end up poll()ing on a stale or wrong fd in case another part of the
application (or another thread) triggered a fatal error.
Getting no data from the socket is not an error condition. This may
happen in case of calling prepare_read() and then read_events() with
no other pending readers and no data in the socket. In general,
read_events() may not queue up events in the given event queue. From
a given threads point of view it doesn't matter whether events were
read and put in a different event queue or no events were read at all.
If EOF is encountered while reading from the Wayland socket, make
wl_display_read_events() return -1 so that it will be treated as an
error. The documentation for this function states that it will set
errno when there is an error so it additionally makes up an errno of
EPIPE.
If we don't do this then when the compositor quits the Wayland socket
will be become ready for reading but wl_display_dispatch will do
nothing which typically makes the application take up 100% CPU. In
particular eglSwapBuffers will likely get stuck in an infinite busy
loop because it repeatedly calls wl_display_dispatch_queue while it
waits for the frame callback.
https://bugzilla.gnome.org/show_bug.cgi?id=703892
With the work to add wl_resource accessors and port weston to use them,
we're ready to make wl_resource and wl_object opaque structs. We keep
wl_buffer in the header for EGL stacks to use, but don't expose it by
default. In time we'll remove it completely, but for now it provides a
transition paths for code that still uses wl_buffer.
Reviewed-by: Jason Ekstrand<jason@jlekstrand.net>
The current thread model assumes that the application or toolkit will have
one thread that either polls the display fd and dispatches events or just
dispatches in a loop. Only this main thread will read from the fd while
all other threads will block on a pthread condition and expect the main
thread to deliver events to them.
This turns out to be too restrictive. We can't assume that there
always will be a thread like that. Qt QML threaded rendering will
block the main thread on a condition that's signaled by a rendering
thread after it finishes rendering. This leads to a deadlock when the
rendering threads blocks in eglSwapBuffers(), and the main thread is
waiting on the condition. Another problematic use case is with games
that has a rendering thread for a splash screen while the main thread
is busy loading game data or compiling shaders. The main thread isn't
responsive and ends up blocking eglSwapBuffers() in the rendering thread.
We also can't assume that there will be only one thread polling on the
file descriptor. A valid use case is a thread receiving data from a
custom wayland interface as well as a device fd or network socket.
The thread may want to wait on either events from the wayland
interface or data from the fd, in which case it needs to poll on both
the wayland display fd and the device/network fd.
The solution seems pretty straightforward: just let all threads read
from the fd. However, the main-thread restriction was introduced to
avoid a race. Simplified, main loops will do something like this:
wl_display_dispatch_pending(display);
/* Race here if other thread reads from fd and places events
* in main eent queue. We go to sleep in poll while sitting on
* events that may stall the application if not dispatched. */
poll(fds, nfds, -1);
/* Race here if other thread reads and doesn't queue any
* events for main queue. wl_display_dispatch() below will block
* trying to read from the fd, while other fds in the mainloop
* are ignored. */
wl_display_dispatch(display);
The restriction that only the main thread can read from the fd avoids
these races, but has the problems described above.
This patch introduces new API to solve both problems. We add
int wl_display_prepare_read(struct wl_display *display);
and
int wl_display_read_events(struct wl_display *display);
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().
From the point of view of a single thread, this ensures that between
calling wl_display_prepare_read() and wl_display_read_events(), no
other thread will read from the fd and queue events in its event
queue. This avoids the race conditions described above, and we avoid
relying on any one thread to be available to read events.
The implementation in this commit allows for one bit worth of flags. If
more flags are desired at a future date, then the wl_map implementation
will have to change but the wl_map API will not.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
The original wl_map implementation did no checking to ensures that ids fell
on the correct side of the WL_SERVER_ID_START line. This meant that a
client could send the server a server ID and it would happily try to use
it. Also, there was no distinction between server-side and client-side in
wl_map_remove. Because wl_map_remove added the entry to the free list
regardless of which side it came from, the following set of actions would
break the map:
1. Client creates a bunch of objects
2. Client deletes one or more of those objects
3. Client does something that causes the server to create an object
Because of the problem in wl_map_remove, the server would take an old
client-side id, apply the WL_SERVER_ID_START offset, and try to use it as a
server-side id regardless of whether or not it was valid.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
