Currently Xwayland is cleaned up by a destroy listener. The problem with
this is that this is true for both libweston's Xwayland support as well
as the frontend's.
Add an explicit destroy step to Xwayland frontend which will cleanly
destroy the process as well as any other resources.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Return a void * from wet_load_xwayland, so we can later pass it back to
explicitly call the cleanup.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Remove all handling of process/PID internals from libweston's Xwayland
launcher, and keep this only in the frontend. libweston now only sees
the wl_client and nothing else.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Every time we call wet_client_launch, we now allocate a new wet_process,
which is always cleaned up by the compositor core and not by the users.
In doing this, weston_client_launch is renamed to wet_client_launch,
since wet_ is for the frontend and weston_ is for libweston.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Instead of reusing an inline wet_process struct, allocate a new
wet_process every time we go to launch Xwayland.
Signed-off-by: Daniel Stone <daniels@collabora.com>
wet_process provides a cleanup function which can be called, but only
passes the process itself. This relies on the process struct being
inlined in something else meaningful, and means that we can't allocate
them on demand.
Add a 'data' argument which allows users to pass meaningful data to
their cleanup handler.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Now that our process-launching internals are identical between the
(still-misnamed) weston_client_launch and the frontend's Xwayland
launcher, we can reuse the internals instead of open-coding it.
As a result, we now additionally prevent Xwayland from inheriting
Weston's signal mask, by clearing SIG_UNBLOCK on all signals. This
should have no observable effect as we do not depend on signal handling
within Xwayland, instead using the displayfd readiness mechanism since
c2f4201ed2.
Signed-off-by: Daniel Stone <daniels@collabora.com>
This gets us closer to the implementation of weston_client_launch, so we
can reuse that instead of open-coding it.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Fixes the following warnings when building with _FORTIFY_SOURCE
and optimizations enabled:
../shared/xalloc.h:49:9: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
49 | write(STDERR_FILENO, oommsg, strlen(oommsg));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
or
../compositor/main.c:427:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
427 | write(STDERR_FILENO, fail_seteuid,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
428 | strlen(fail_seteuid));
| ~~~~~~~~~~~~~~~~~~~~~
../compositor/main.c:434:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
434 | write(STDERR_FILENO, fail_cloexec,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
435 | strlen(fail_cloexec));
| ~~~~~~~~~~~~~~~~~~~~~
../compositor/main.c:442:25: error: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
442 | write(STDERR_FILENO, fail_exec, strlen(fail_exec));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
If we leave xwayland in weston's process group, it can receive
signals from the controlling TTY intended for weston.
The easiest way to see this is to launch weston under gdb, start an
X client, and hit ctrl-c in the gdb session. The Xwayland server
will also catch the SIGINT, and the X client will be disconnected.
Instead, let's call setsid() when launching Xwayland, like we do
for launched clients.
Suggested-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Use the arg handling added in the previous commit so that the
environment is completely encapsulated inside the custom env.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Rename the bits handling environment variables (currently, all of it),
so we have room to handle args as well.
No functional changes.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. Painfully, setenv() is not listed as such.
Therefore we must craft our own custom environment, and we get no help
from libc with that.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Constructing argv before-hand is a little easier to look at, but this is
mostly just anticipating more changes to how Weston spawns processes in
general.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We are already using pipe2() in many places, even in libweston, so let's
simplify the code here as well - not to mention avoid a theoretical
race.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. Surprisingly, snprintf() is not such
function. See e.g. https://stackoverflow.com/a/6771799 and how snprintf
is not listed in signal-safety(7) manual.
Therefore we must prepare the fd argument strings before fork(). That is
only possible if we also do not dup() fd in the child process. Hence we
remove the close-on-exec flag instead in the child process which has
copies of the parent's file descriptors. Fortunately fcntl() is safe.
struct fdstr is helping to reduce code clutter a bit.
Additionally, if fork() fails, we now clean up the fds we created.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Between fork() and exec() in the child process it is only safe to use
async-signal-safe functions. weston_log() definitely is not one, it
allocates memory and does whatnot.
weston_log() is also inappropriate for other reasons: the child process
has its own stream buffers and flight-recorder. No-one looks into the
child process' flight recorder, so messages would be lost there. The
logging machinery might also attempt to write into debug streams,
meaning both parent and child could be writing simultaneously.
It seems that the best we can do is to pre-bake an error message and
only write() it out if exec() fails. There is no mention that even
strerror_r() might be safe to call, so we don't.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Doing any kind of memory allocation calls between fork() and exec() in
the child process is prone to deadlocks and explosions. In general, only
async-signal-safe functions are safe there.
Move the config access to the parent process before fork() to avoid
problems.
See also:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/941#note_1457053
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This reverts commit 4aa885d4af.
Turns out the problem was not about dupping fds at all, but calling
non-async-signal-safe functions like strdup() between fork() and exec()
in the child process.
For more discussion, see:
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/941#note_1457053
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Replace an oft-duplicated pattern with a trivial helper function. In
doing so, we observe that the one special case (displayfd 'didn't need
to be CLOEXEC') was wrong, because the X server does fork itself
internally, so there is nothing wrong with setting CLOEXEC.
Signed-off-by: Daniel Stone <daniels@collabora.com>
We want to wait for Xwayland to be ready before issuing it blocking
requests, but relying on USR1 is a bit unsafe:
- we can't ascertain the signal originated from Xwayland
- if weston is started as PID1 (e.g. in its own container), then
Xwayland will not send SIGUSR1 and X11 connections will be stuck
forever: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1312
Creating a pipe and using -displayfd, even if we don't care about the
display value itself, is safe and works for all cases
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
We can use it just once to define a string instead of having preprocessor
conditionals sprinkled about the code.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
The printf() format specifier "%m" is a glibc extension to print
the string returned by strerror(errno). While supported by other
libraries (e.g. uClibc and musl), it is not widely portable.
In Weston code the format string is often passed to a logging
function that calls other syscalls before the conversion of "%m"
takes place. If one of such syscall modifies the value in errno,
the conversion of "%m" will incorrectly report the error string
corresponding to the new value of errno.
Remove all the occurrences of the specifier "%m" in Weston code
by using directly the string returned by strerror(errno).
While there, fix some minor indentation issue.
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
The main idea is to make libweston users use the form
#include <libweston/libweston.h>
instead of the plain
#include <compositor.h>
which is prone to name conflicts. This is reflected both in the installed
files, and the internal header search paths so that Weston would use the exact
same form as an external project using libweston would.
The public headers are moved under a new top-level directory include/ to make
them clearly stand out as special (public API).
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We are currently formatting 32-bit signed integers into 8 byte buffers,
which are too small, causing the compiler to complain. Update the buffer
size to the minimum required value of 12 bytes: 1 for the sign, 10 for
the number, and 1 for the null byte terminator.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
This patch follows a similar approach taken to detach the backends from
weston. But instead of passing a configuration struct when loading the
plugin, we use the plugin API registry to register an API, and to get it
in the compositor side. This API allows to spawn the Xwayland process
in the compositor side, and to deal with signal handling. A new
function is added in compositor.c to load and init the xwayland.so
plugin.
Also make sure to re-arm the SIGUSR1 when the X server quits.
Signed-off-by: Giulio Camuffo <giuliocamuffo@gmail.com>
[Pekka: moved xwayland/weston-xwayland.c -> compositor/xwayland.c]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
