To fix a shutdown crash in weston's x11 compositor I want to move the
weston X window close to an idle handler.
Since idle handlers are processed at the start of an event loop, the
handler that deals with window close will run at the start of the
next input_loop dispatch, after which the dispatcher blocks on epoll
forever (since all input events that will ever occur have been consumed).
Dispatching idle callbacks both at the start and end of event-loop
processing will prevent this permanent blocking.
Note that just moving the callback dispatch could theoretically
result in an idle callback being delayed indefinitely while waiting
for epoll_wait() to complete.
Callbacks are removed from the list when they're run, so the second
dispatch won't result in any extra calls.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Giulio Camuffo <giuliocamuffo@gmail.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
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>
Server protocols headers should include wayland-server.h,
instead of wayland-util.h. Otherwise they're not useable
with C++ compiler unless wayland-server.h was included
earlier.
Signed-off-by: Mariusz Ceier <mceier+wayland@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
There are functions below the "Deprecated functions below" comment
that are not deprecated.
Move the deprecated functions back down, and add a comment at the
end of the file to try to keep this from happening again.
Signed-off-by: Derek Foreman <derekf@osg.samsung.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>
This object is only in wayland-private.h so it's methods should not
be in the documentation.
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
We already have the id variable there and it makes it slightly easier to
read.
Signed-off-by: Jonas Ådahl <jadahl@gmail.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 is a minor documentation fix. I did not see any asterisks in the
output as reported by Pekka Paalanen. Using doxygen 1.7.6.1.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
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>
Since commit 4c163b9b00, wayland-scanner
is built in top builddir instead of src, and protocol files are
generated in protocol subdir instead of src.
Protocol files generated in the new path are already properly ignored
in the toplevel gitignore file.
Signed-off-by: Olivier Blin <olivier.blin@softathome.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Quells a doxygen warning:
src/wayland-server.c:790: warning: argument 'None' of command @param is
not found in the argument list of wl_display::wl_display_create(void)
Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
This happens on EOF if using a poll function such as select() or
kqueue() which doesn’t distinguish EOF events.
Currently execution should never reach the point where recvmsg() returns
EOF (len == 0). Instead, epoll() will detect this and indicate EPOLLHUP,
which is handled a few lines above, closing the connection. However,
other event mechanisms may not be able to distinguish EOF from regular
readability (in the case of select()) or inconsistently across platforms
(in the case of POLLHUP). There is also the possibility of half-closed
connections (shutdown(), POLLRDHUP), though this may not be an issue
with Wayland.
This will not cause problems if the FD polls as readable but actually is
not — in that case, recvmsg() will return EAGAIN.
Signed-off-by: Philip Withnall <philip at tecnocode.co.uk>
Signed-off-by: Karsten Otto <ottoka at posteo.de>
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Signed-off-by: Philip Withnall <philip at tecnocode.co.uk>
Signed-off-by: Karsten Otto <ottoka at posteo.de>
Reviewed-by: David Fort <contact at hardening-consulting.com>
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
It expects ((msg_controllen == 0) == (msg_control == NULL)), and returns
EINVAL otherwise. It can't hurt to be tidy about things on other platforms
either though.
See: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=99356#c5
Signed-off-by: Philip Withnall <philip at tecnocode.co.uk>
Signed-off-by: Karsten Otto <ottoka at posteo.de>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Calling close() on the same file descriptor that a previous call to
close() already closed is wrong, and racy if another thread received
that same file descriptor as a eg. new socket or actual file.
There are two situations where wl_connection_destroy() would close its
file descriptor and then another function up in the call chain would
close the same file descriptor:
* When wl_client_create() fails after calling wl_connection_create(),
it will call wl_connection_destroy() before returning. However, its
caller will always close the file descriptor if wl_client_create()
fails.
* wl_display_disconnect() unconditionally closes the display file
descriptor and also calls wl_connection_destroy().
So these two seem to expect wl_connection_destroy() to leave the file
descriptor open. The other caller of wl_connection_destroy(),
wl_client_destroy(), does however expect wl_connection_destroy() to
close its file descriptor, alas.
This patch changes wl_connection_destroy() to indulge this majority of
two callers by simply not closing the file descriptor. For the benefit
of wl_client_destroy(), wl_connection_destroy() then returns the
unclosed file descriptor so that wl_client_destroy() can close it
itself.
Since wl_connection_destroy() is a private function called from few
places, changing its semantics seemed like the more expedient way to
address the double-close() problem than shuffling around the logic in
wl_client_create() to somehow enable it to always avoid calling
wl_connection_destroy().
Signed-off-by: Benjamin Herr <ben@0x539.de>
Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
These blocks were misformatted in normal paragraph style in the
generated docs. Also, added \comment{} for comments within one code
example.
Signed-off-by: Benjamin Herr <ben@0x539.de>
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>
