This reverts commit baf92d09d6.
Closes#787
According to the original commit, this change was made because otherwise
g_test_dbus_down() following a g_test_dbus_stop() hangs until it times
out. The timeout being referred to is the 30 seconds which are waited by
_g_object_unref_and_wait_weak_notify() for the GWeakNotify to be
triggered when the last strong reference to the singleton
GDBusConnection object is dropped. But the patch was not correct and the
leak should have instead been fixed by having the last strong reference
holder drop their reference on the GDBusConnection before calling
g_test_dbus_down(). Timing out after 30 seconds is the desired behavior
in the case where someone holds a reference to the singleton for that
entire period.
There are a few problems with this patch. First, as pointed out here[1],
calling g_object_run_dispose() in the idle callback means we are causing
the GWeakNotify to trigger ~immediately rather than waiting 30 seconds
to give another owner a chance to unref. Second, since someone else may
still hold a reference on the object being disposed, they may call
methods on it after it's been disposed which can seg fault as documented
here[2] and as I also saw recently in another project.
It's unclear what the original leak being fixed was, but many have been
fixed between 2013 and now. I ran all the unit tests under valgrind, and
some do fail (some consistently and some intermittently) but none of the
failures seem to only happen after this reversion commit. I also
couldn't find anywhere in the valgrind output where any GDBusConnection
objects are definitely being lost.
[1] https://gitlab.gnome.org/GNOME/glib/issues/787#note_214226
[2] https://gitlab.gnome.org/GNOME/glib/issues/787#note_214237
This (dubious) testcase fails before the previous commit due to errno
being clobbered by the interrupted wait on the contended mutex. The
previous commit fixes that.
The testcase is dubious because, in theory (as per POSIX),
g_cond_wait_until() is permitted to return TRUE at any time for any
reason, due to so-called "spurious wakeups". Having a testcase that
asserts that the return value should be FALSE is therefore fundamentally
broken. We do it anyway, though.
We're only really trying to test a bug in our homemade Linux/futex
implementation here, and it takes a fair amount of effort to actually
convince the old code to fail (including some system stuff which
probably isn't available on Windows). There's also the spurious wakeup
situation mentioned above to worry about on other systems. For all of
those reasons, this test is only enabled on Linux.
The return value from `g_cond_wait_until()` is calculated, based on the
value of `errno` after reacquiring the mutex. This is a problem because
`errno` can be overwritten in the case the mutex is contended (in which
case the slow-path code will re-enter the kernel).
Perform the calculation before reacquiring the mutex.
See merge request GNOME/glib!958
Visual Studio 2010 and later support static_assert on both C and C++,
but we can only enable it for plain-C code on Visual Studio 2013 and
later, as apparently the static_assert macro implementation will carry
out something after defining certain variables.
For C++ code, we use static_assert on Visual Studio 2010 and later.
The return value for g_utf8_normalize may be null, but the return type
is not annotated as such. This is important for language bindings for
langs that are about nullability, such as Vala and Haskell.
g_test_trap_fork() is deprecated, but it’s still tested in the GLib
tests. If we don’t set the coredump rlimit to 0 to prevent coredumps,
the test for it creates a coredump (which is expected). On systems with
systemd-coredump, that causes systemd-coredump to spawn and process the
coredump for a while. That slows down the tests, and results in spurious
coredump lists in the system logs.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #436
For several years now (I haven’t looked up the exact date),
`gnome-terminal` has preferred being called as `gnome-terminal
--terminal-args -- /some/other/program --its-args` rather than as
`gnome-terminal --terminal-args -x /some/other/program --its-args`.
Since 2017 it has warned about uses of `-x` (see
ad4edbd118).
So we should change our calling convention for it.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
There seems to be no reason to do so, and since the `appinfo` test was
ported to use `G_TEST_OPTION_ISOLATE_DIRS`, it has been causing
coredumps to accumulate. `gnome-terminal` was chosen as the terminal,
but it couldn’t find its GSettings schemas due to all the XDG
environment variables being cleared to `/dev/null` by
`G_TEST_OPTION_ISOLATE_DIRS`.
In order to keep using `gnome-terminal` as a subprocess in the tests,
we’d need to explicitly set up its environment so it can load the right
GSettings schemas. That’s a lot of work for not much gain.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #436
The static assert message is much nicer to read, and is less likely to
be misinterpreted as a false positive.
glib is built with `-std=gnu89`, but this macro will be available to
projects that use glib with c11 or gnu11.
LoadLibrary() is not available when building for the Universal Windows
Platform, which is used for shipping apps to the Windows Store on all
devices (Windows Desktop, Windows Phone, Surface, XBox, etc).
Apps are not allowed to load arbitrary DLLs from the system. The only
DLLs they can load are those that are bundled with the app as assets.
LoadPackagedLibrary() can be used to access those assets by filename.
The function is meant to be a drop-in replacement for LoadLibrary(),
and the HANDLE returned can be treated the same as before.
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.
The two test scripts actually assumed some *NIX paradigms, so we need
to adapt them so that they can work on Windows as well, the changes are
namely:
-Call the glib-mkenums and glib-genmarshal Python scripts with the
Python interpreter, not just relying on shebang lines, on Windows.
This is because the native Windows console (cmd.exe) does not support
shebang lines, for subprocess.run().
-Use NamedTemporaryFile with delete=False, otherwise Windows cannot find
the temp files we need when running the tests.
-Use universal_newlines=True for subprocess.run() so that we do not need
to worry out line ending differences on different systems.
-Make sure we are not in the temp directories we create, where the tests
are being run, upon cleanup. Windows does not like deleting
directories that we are currently in.
On Windows and possibly other platforms the '%p' printf modifier does
not prefix printed values with '0x', so do not expect the warning
message to contain the '0x' prefix for the handler pointer value.
It is unclear that why the size of the buffer was chosen to be 100
because the commit introduced the code didn't mention the reason.
POSIX defines _POSIX_HOST_NAME_MAX to be 255 and provides a way to
determine the suitable value with sysconf, so we should use it instead
of hard-coding a small value.
On Visual Studio, Meson builds modules as xxxx.dll, not libxxxx.dll when
xxxx is specified as the name for the shared_module() build directive.
This means that in the test programs if we expect for libxxxx for the
module name, the test will fail as there is no libxxxx.dll but there is
xxxx.dll. This makes the test program look for the module files
correctly.
