This commit changes a comment in _g_dbus_worker_do_read_cb() to be
slightly more useful. At least in my experience debugging an
intermittent unit test failure in another project, this failure
condition occurred because although g_test_dbus_down() ensures that the
session GDBusConnection has exit-on-close set to FALSE before killing
its dbus-daemon, there was still a GDBusConnection on the system bus
which hit this failed read code path, because we had
DBUS_SYSTEM_BUS_ADDRESS set to the address of the #GTestDBus daemon, to
appease libudisks.
Also, make a few other minor improvements to the docs.
In file gio/gtestdbus.c, function watch_parent, there is a loop which
waits for commands sent from the parent process and kills all processes
recorded in 'pids_to_kill' array on parent process exit. The detection
of parent process exit is done by calling g_poll and checking whether
the returned event is G_IO_HUP. However, 'revents' is a bit mask, and
we should use a bitwise-AND check instead of the equality check here.
It seems to work fine on Linux, but it fails on FreeBSD because the
g_poll returns both G_IO_IN and G_IO_HUP on pipe close. This means the
watcher process continues waiting for commands after the parent process
exit, and g_io_channel_read_line returns G_IO_STATUS_EOF with 'command'
set to NULL. Then the watcher process crashes with segfault when calling
sscanf because 'command' is NULL. Since the test result is already
reported by the parent process as 'OK', this kind of crash is likely to
be unnoticed unless someone checks dmesg messages after the test:
pid 57611 (defaultvalue), uid 1001: exited on signal 11
pid 57935 (actions), uid 1001: exited on signal 11
pid 57945 (gdbus-bz627724), uid 1001: exited on signal 11
pid 57952 (gdbus-connection), uid 1001: exited on signal 11
pid 57970 (gdbus-connection-lo), uid 1001: exited on signal 11
pid 57976 (gdbus-connection-sl), uid 1001: exited on signal 11
pid 58039 (gdbus-exit-on-close), uid 1001: exited on signal 11
pid 58043 (gdbus-exit-on-close), uid 1001: exited on signal 11
pid 58047 (gdbus-exit-on-close), uid 1001: exited on signal 11
pid 58051 (gdbus-exit-on-close), uid 1001: exited on signal 11
pid 58055 (gdbus-export), uid 1001: exited on signal 11
pid 58059 (gdbus-introspection), uid 1001: exited on signal 11
pid 58065 (gdbus-names), uid 1001: exited on signal 11
pid 58071 (gdbus-proxy), uid 1001: exited on signal 11
pid 58079 (gdbus-proxy-threads), uid 1001: exited on signal 11
pid 58083 (gdbus-proxy-well-kn), uid 1001: exited on signal 11
pid 58091 (gdbus-test-codegen), uid 1001: exited on signal 11
pid 58095 (gdbus-threading), uid 1001: exited on signal 11
pid 58104 (gmenumodel), uid 1001: exited on signal 11
pid 58108 (gnotification), uid 1001: exited on signal 11
pid 58112 (gdbus-test-codegen-), uid 1001: exited on signal 11
pid 58116 (gapplication), uid 1001: exited on signal 11
pid 58132 (dbus-appinfo), uid 1001: exited on signal 11
If the watcher process crashes before killing the dbus-daemon process
spawned by the parent process, the dbus-daemon process will keep running
after all tests complete. Due to the implementation of 'communicate'
function in Python subprocess, it causes meson to crash. 'communicate'
assumes the stdout and stderr pipes are closed when the child process
exits, but it is not true if processes forked by the child process
doesn't exit. It causes Python subprocess 'communicate' function to
block on the call to poll until the timeout expires even if the test
finishes in a few seconds. Meson assumes the timeout exception always
means the test is still running. It calls 'communicate' again and
crashes because pipes no longer exist.
https://gitlab.gnome.org/Infrastructure/GitLab/issues/286https://github.com/mesonbuild/meson/issues/3967https://bugs.python.org/issue30154
All those logging functions already add a newline to any message they
print, so there’s no need to add a trailing newline in the message
passed to them.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: nobody
There's a race condition somewhere in GTestDBus that can result in
the next test being started at a time when g_bus_get() would still
return the connection that is in the process of closing. This can
be reproduced reasonably reliably by running the gapplication test
10K times in a loop.
Instead of relying on waiting for the weak reference to be released,
we can force the issue by clearing it.
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=768996
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=894677
Prevent the situation where errno is set by function A, then function B
is called (which is typically _(), but could be anything else) and it
overwrites errno, then errno is checked by the caller.
errno is a horrific API, and we need to be careful to save its value as
soon as a function call (which might set it) returns. i.e. Follow the
pattern:
int errsv, ret;
ret = some_call_which_might_set_errno ();
errsv = errno;
if (ret < 0)
puts (strerror (errsv));
This patch implements that pattern throughout GLib. There might be a few
places in the test code which still use errno directly. They should be
ported as necessary. It doesn’t modify all the call sites like this:
if (some_call_which_might_set_errno () && errno == ESOMETHING)
since the refactoring involved is probably more harmful than beneficial
there. It does, however, refactor other call sites regardless of whether
they were originally buggy.
https://bugzilla.gnome.org/show_bug.cgi?id=785577
If we have an input parameter (or return value) we need to use (nullable).
However, if it is an (inout) or (out) parameter, (optional) is sufficient.
It looks like (nullable) could be used for everything according to the
Annotation documentation, but (optional) is more specific.
This is the right thing to do for the "a session is a user-session"
model implemented in dbus 1.9.14, which is described in
<http://lists.freedesktop.org/archives/dbus/2015-January/016522.html>.
It also resembles sd-bus' behaviour, although sd-bus will only try
kdbus and XDG_RUNTIME_DIR/bus, and never runs dbus-launch.
On systems following the more traditional "a session is a login-session"
model, X_R_D/bus won't exist, so it is harmless to check for it before
falling back to X11 autolaunching. Again, this matches the behaviour
of current libdbus and sd-bus versions.
Now that we do this, g_test_dbus_unset() needs to clear XDG_RUNTIME_DIR
as well as everything else.
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=747941
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
This avoids any possibility of interfering with test syntax (such as
TAP) on stdout. TAP specifically does not parse stderr.
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=725981
Reviewed-by: Colin Walters <walters@verbum.org>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
The stdout FD passed to dbus-daemon is propagated to all its child
processes, such as service activated processes. If we close the FD after
reading the bus address from the daemon, any child process which
subsequently writes to stdout (e.g. for logging) will get a SIGPIPE and
explode.
Instead of closing the stdout FD immediately after dbus-daemon has
spawned, keep it open until the daemon is killed.
https://bugzilla.gnome.org/show_bug.cgi?id=732019
Windows does not like g_unlink() to be called on files whose file
descriptor is still open, so doing that would cause a permission
denied error. Since the fd is not used in that function after
acquiring the temp file, close it earlier before
g_file_set_contents(), so that it can complete successfully.
This fixes a number of GTK+ tests on Windows.
https://bugzilla.gnome.org/show_bug.cgi?id=719344
In Windows development environments that have it, <unistd.h> is mostly
just a wrapper around several other native headers (in particular,
<io.h>, which contains read(), close(), etc, and <process.h>, which
contains getpid()). But given that some Windows dev environments don't
have <unistd.h>, everything that uses those functions on Windows
already needed to include the correct Windows header as well, and so
there is never any point to including <unistd.h> on Windows.
Also, remove some <unistd.h> includes (and a few others) that were
unnecessary even on unix.
https://bugzilla.gnome.org/show_bug.cgi?id=710519
As it turns out, we have examples of internal functions called
type_name_get_private() in the wild (especially among older libraries),
so we need to use a name for the per-instance private data getter
function that hopefully won't conflict with anything.
Rather than defining _WIN32_WINNT only in a handful of files, define
it in config.h, like we do with _GNU_SOURCE.
(Also remove a "#define WIN32_LEAN_AND_MEAN" that isn't really all
that useful.)
https://bugzilla.gnome.org/show_bug.cgi?id=688109
-glib/gmarkup.c: Use G_VA_COPY() instead of va_copy() as va_copy() may not
be universally available.
-gio/gtestdbus.c: Include io.h on Windows for close()
Using GIO here may cause the gvfs module to be loaded, which
in turn gets onto the session bus to talk to gvfsd - not ideal
if you are trying to control the session bus life cycle. Instead,
just use old-fashioned glib file utils.
-gconverterinputstream.c: Avoid GCCism by not using non-standard pointer
arithmetic on void*, but do a cast to char * as that seems to be what the
variable was used for.
-gtestdbus.c: Don't include unistd.h unconditionally, and use g_usleep()
instead of usleep(), as usleep() is not universally available.
