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>
The idea here was that once upon a time, clients could rebind wl_display
to a higher version, so we offered the ability to rebind it
here. However, this is particularly broken. The existing bind
implementation actually still hardcodes version numbers, and it leaks
previous resources, overwriting the existing one.
The newly bound resource *also* won't have any listeners attached by the
client, meaning that the error and delete_id events won't get delivered
correctly. Unless the client poked into libwayland internals, it also
can't possibly set up these handlers correctly either, so the client
will sustain errors and leak all deleted globals.
Since this never worked correctly in the first place, we can feel safe
removing it.
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
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 we add more that one source to a signal, then wayland will
block in wl_event_loop_dispatch. This is due to the attampt to read
from signal's fd each time the source is dispatched.
wl_event_loop_add_signal(loop, SIGINT, ...);
wl_event_loop_add_signal(loop, SIGINT, ...);
/* raise signal .. */
/* we got two fd's ready, both for the one SIGINT */
epoll_wait(...) = 2
[ for (i == 0) ]
source1->dispatch() --> read(fd1);
[ for (i == 1) ]
source2->dispatch() --> read(fd2); /* blocking! */
Reading from fd2 will block, because we got only one signal,
and it was read from fd1.
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
"is_interface" is a really terrible name for the client or server
variants, and instead of checking whether we were passed the requests or
the events, just pass an argument through.
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
0 is also a valid fd, and needs to be closed.
On error we set fd to -1. We need to also initialize fds to -1, so we do
not accidentally close stdout on error.
While fixing this, also remove one use-before-NULL-check.
Based on the patch by Marek.
Cc: Marek Chalupa <mchqwerty@gmail.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Jasper St. Pierre <jstpierre@mecheye.net>
When some function during adding socket fails, it must clean
everything it set or we can get funky errors.
This patch fixes:
http://lists.freedesktop.org/archives/wayland-devel/2014-August/016331.html
Signed-off-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
The code here is wrong, leaky, and inconsistent. We don't free,
unlink or clean up things when we should in every error path.
Centralize the data destruction so it's easier to keep track of
and easier to bug fix.
Use function linking syntax instead of variable linking, to resolve two
warnings:
wayland-server.h:167: warning: explicit link request to 'wl_list_remove' could not be resolved
wayland-server.h:188: warning: explicit link request to 'wl_list_remove' could not be resolved
Signed-off-by: Bryce Harrington <b.harrington@samsung.com>
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>
Commit 99a72777f9 introduced a new error
for when the 'since' version decreases. It also reset the version for
messages without a version to 1. Versioning semantics in the spec files
was a little under-specified and we don't want to break projects caught in
this grey zone.
This commits replaces previous configure.ac as the 1.4.93 tag and the
final 1.5 RC.
This could be useful for compositors who need to be able to not send
events if the client bound a version lower than the newest provided.
Event version numbers are exposed as
[INTERFACE_NAME]_[EVENT_NAME]_SINCE_VERSION for example wl_output.scale
will have the version macro WL_OUTPUT_SCALE_SINCE_VERSION.
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
Fail if a message with version implicitly set to 1 (i.e. not specified)
comes after a message with since-version > 1.
Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
If for some reason that errno is neither value (ENOMEM or
EINVAL), then prior to this patch, there would be a NULL
deref in wl_closure_lookup(...) at the "else if" conditional
when closure == NULL. Also, closure might not be NULL but still
fall into the block due to the wl_closure_lookup < 0 condition...
in that case, we need to destroy the closure to avoid a memory
leak.
Currently, wl_connection_demarshal only sets errno to ENOMEM
or EINVAL... we've already checked for ENOMEM so remove check
for EINVAL (just assume it). Also, call wl_closure_destroy(...)
unconditionally in the "else if" block (assume it can handle
NULL closure, too, which it does right now).
Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
This implements a simple fix for the blocking problem that occurs when
updating a timer event source after the timer expires, but before its
callback is dispatched. This can happen when another event happens during the
same epoll wakeup as the timer event, and causes the read() call in
wl_event_source_timer_dispatch() to block for the updated duration of the
timer.
We never want this read() call to block, so I believe it makes sense for the
timerfd to be non-blocking, and we simply ignore the case where the read fails
with EAGAIN. We still report all other errors as before, and still ignore the
actual value read from the socket.
With this change, the event_loop_timer_updates unit test case I submitted
previously now passes, and weston appears to work as before.
If a message was too big to fit in the connection buffer, the code
in wl_buffer_put would just write past the end of it.
I haven't seen any real world use case that would trigger this bug, but
it was possible to trigger it by sending a long enough string to the
wl_data_source.offer request.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69267
In order to set a logging function all the time, the output we get
needs to be useful. Logging about trivial things like the socket
we're using and when clients disconnect doesn't realy help anyone.
In some cases, like Xwayland, stdout and stderr are redirected to
/dev/null, losing us valuable information, while wl_log can be
overridden, allowing us to send it to a log file instead. This
can help debugging immensely.
The code very intentionally emits a lot of redundant declarations
to simplify the scanner code. Somebody building with -Wredundant-decls
would have compile errors, so emit special pragmas to turn those
warnings off.
These pragmas should be ignored outside of gcc/clang.
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.
The previous implementation of the wl_container_of macro was
dereferencing the sample pointer in order to get an address of the
member to calculate the offset. Ideally this shouldn't cause any
problems because the dereference doesn't actually cause the address to
be read from so it shouldn't matter if the pointer is uninitialised.
However this is probably technically invalid and could cause undefined
behavior. Clang appears to take advantage of this undefined behavior
and doesn't bother doing the subtraction. It also gives a warning when
it does this.
The documentation for wl_container_of implies that it should only be
given an initialised pointer and if that is done then there is no
problem with clang. However this is quite easy to forget and doesn't
cause any problems or warnings with gcc so it's quite easy to
accidentally break clang.
To fix the problem this changes the macro to use pointer -
offsetof(__typeof__(sample), member) so that it doesn't need to deref
the sample pointer. This does however require that the __typeof__
operator is supported by the compiler. In practice we probably only
care about gcc and clang and both of these happily support the
operator.
The previous implementation was also using __typeof__ but it had a
fallback path avoiding it when the operator isn't available. The
fallback effectively has undefined behaviour and it is targetting
unknown compilers so it is probably not a good idea to leave it in.
Instead, this patch just removes it. If someone finds a compiler that
doesn't have __typeof__ but does work with the old implementation then
maybe they could add it back in as a special case.
This patch removes the initialisation anywhere where the sample
pointer was being unitialised before using wl_container_of. The
documentation for the macro has also been updated to specify that this
is OK.
