Track the `GTask`s which are still alive (not finalised) in a shared
list, and provide a secret debugging function for printing that list.
Too often when debugging apps, I have found that a ‘leaked’ object is
actually still (validly) referenced by an ongoing `GTask` which hasn’t
completed for whatever reason. Or I have found that an operation has
obviously stalled, but there are no pointers available to the `GTask`
which is stalled, because it’s being tracked as a collection of closure
pointers from some `GSource` which is hard to get to in the debugger.
It will be very useful for debugging apps, if there’s a list of all the
still alive `GTask`s somewhere. This is that list.
The code is disabled if `G_ENABLE_DEBUG` is not defined, to avoid every
`GTask` construction/finalisation imposing a global locking penalty.
To use the new list, break in `gdb` while running your app, and call
`g_task_print_alive_tasks()`, or inspect the `task_list` manually:
```
(gdb) print g_task_print_alive_tasks()
16:44:17:788 GLib-GIO 5 GTasks still alive:
• GTask 0x6100000ac740, gs_plugin_appstream_setup_async, ref count: 1, ever_returned: 0, completed: 0
• GTask 0x6100000bf940, [gio] D-Bus read, ref count: 2, ever_returned: 0, completed: 0
• GTask 0x6100000aac40, gs_plugin_loader_setup_async, ref count: 1, ever_returned: 0, completed: 0
• GTask 0x61000006d940, gs_plugin_loader_job_process_async GsPluginJobRefine, ref count: 1, ever_returned: 0, completed: 0
• GTask 0x610000118c40, [gio] D-Bus read, ref count: 2, ever_returned: 0, completed: 0
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This commit changes the use of `ngettext` with `g_dngettext`. The
project defined `g_dngettext` (with domain support) provides the same
functionality as `ngettext` with a NULL domain provided. The purpose of
this change is to help address a build error for certain compilers that
trigger a `format-nonliteral` error-promoted-warning when using
`ngettext` (see also [1][2]). The benefit of switching to use
`g_dngettext` is that the function is defined with `G_GNUC_FORMAT`. This
provides a hint to GNU GCC compilers to still sanity check these
arguments, but not generate a `format-nonliteral`.
[1]: 4ae8606b6f
[2]: 0ca660315a
Signed-off-by: James Knight <james.d.knight@live.com>
This reverts commit 4ae8606b6f. The idea
for the change [1] was to address a build error for certain compilers
that trigger a `format-nonliteral` error-promoted-warning since these
compilers do not gracefully support `ngettext` usage. The changes
following a pattern from an old commit [2]; however, James Hilliard has
pointed out these changes do not work as intended. A deeper inspection
of the commit showed that the commit was from an old merge request that
was not pulled in, detailing why the changes did not work (see also
[3][4]).
Manipulating the sockets unit test confirms that the format values no
longer get a proper value:
...
ok 9 /socket/address
ok 10 /socket/unix-from-fd
ok 11 /socket/unix-connection
**
GLib-GIO:ERROR:../gio/tests/socket.c:1493:test_unix_connection_ancillary_data: assertion failed (err == NULL): Expecting one fd, but got %d
(g-io-error-quark, 0)
...
And reverting this change restores the original functionality:
...
ok 9 /socket/address
ok 10 /socket/unix-from-fd
ok 11 /socket/unix-connection
**
GLib-GIO:ERROR:../gio/tests/socket.c:1493:test_unix_connection_ancillary_data: assertion failed (err == NULL): Expecting 1 control message, got 0 (g-io-error-quark, 0)
...
[1]: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3390
[2]: 44b3d5d80445234041f6c59feb89645f7102c3a4
[3]: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/770
[4]: https://gitlab.gnome.org/GNOME/glib/-/issues/1744
Signed-off-by: James Knight <james.d.knight@live.com>
It’s a bad idea to use it without some care for how much it’s being
called in parallel, or dependencies between tasks. If the thread pool
gets exhausted by too many inter-dependent calls to
`g_task_run_in_thread()` then the process will livelock.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Update a series of error messages to use `g_set_error_literal` instead
of `g_set_error`. This should prevent `format-nonliteral` compiler
issues when `-Werror` is configured:
../gio/gunixconnection.c: In function ‘g_unix_connection_receive_fd’:
../gio/gunixconnection.c:183:9: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
183 | nscm);
| ^~~~
../gio/gunixconnection.c:217:20: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
217 | nfd);
| ^~~
../gio/gunixconnection.c: In function ‘g_unix_connection_receive_credentials’:
../gio/gunixconnection.c:601:24: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
601 | nscm);
| ^~~~
This is similar to a previous change [1] made to `gunixconnection.c`.
[1]: 44b3d5d80445234041f6c59feb89645f7102c3a4
Signed-off-by: James Knight <james.d.knight@live.com>
Since commit c0ca3f99 this test is strictly depending on GDesktopAppInfo
that is not defined or available in macos, so skip the test as we do for
windows.
We could have done this at meson level too, but keeping it this way is
probably a better reminder that this should be adapted for such scenario
one day™
See: https://gitlab.gnome.org/GNOME/glib/-/jobs/2753753
The merge request !2848 added code to automatically detect the module
prefix on macOS, with a test for the Mac #define TARGET_OS_OSX. However,
older versions of the SDK (at least 10.11) don't provide this #define,
leading to build failure. If the #define is missing, fall back to
checking TARGET_OS_MAC. On newer SDKs this symbol is also true for
watchOS, etc., but in those situations TARGET_OS_OSX is available.
Various gio modules include gmodule.h that requires the
gmodule-visibility.h to be already built.
To make this easier, just provide a dependency and use it where we are
building modules that do not depend on libgio_dep (that already includes
that).
Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/2982
The generated gir file marks the size parameter as "out" by default. This is wrong in the context of a caller allocated buffer with a given size. Explicitly marking the size parameter as (in) fixes the issue.
This reverts commit 4cad66580b.
Downgrading the criticals was only temporary. Now we’ve branched for
GLib 2.78, the criticals can be reinstated early this cycle, so people
have the maximum time to fix latent bugs in their code.
Fixes: #2951
The ref on the timeout source owned by `SendMessageData` was being
dropped just after attaching the source to the main context, leaving it
unowned in that struct. That meant the only ref on the source was held
by the `GMainContext` it was attached to.
This ref was dropped when returning `G_SOURCE_REMOVE` from
`send_message_with_reply_timeout_cb()`. Before that happens,
`send_message_data_deliver_error()` is called, which normally calls
`send_message_with_reply_cleanup()` and destroys the source.
However, if `send_message_data_deliver_error()` is called when the
message has already been delivered, calling
`send_message_with_reply_cleanup()` will be skipped. This leaves the
source pointer in `SendMessageData` dangling, which will cause problems
when `g_source_destroy()` is subsequently called on it.
I’m not sure if it’s possible in practice for this situation to occur,
but the code certainly does nothing to prevent it, and it’s easy enough
to avoid by keeping a strong ref on the source in `SendMessageData`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
It already implicitly held a strong ref on its `GTask` values, but
didn’t have a free function set so that they would be automatically
unreffed on removal from the map.
This meant that the functions handling removals from the map,
`on_worker_closed()` (via `cancel_method_on_close()`) and
`send_message_with_reply_cleanup()` had to call unref once more than
they would otherwise.
In `send_message_with_reply_cleanup()`, this behaviour depended on
whether it was called with `remove == TRUE`. If not, it was `(transfer
none)` not `(transfer full)`. This led to bugs in its callers.
For example, this led to a direct leak in `cancel_method_on_close()`, as
it needed to remove tasks from `map_method_serial_to_task`, but called
`send_message_with_reply_cleanup(remove = FALSE)` and erroneously didn’t
call unref an additional time.
Try and simplify it all by setting a `GDestroyNotify` on
`map_method_serial_to_task`’s values, and making the refcount handling
of `send_message_with_reply_cleanup()` not be conditional on its
arguments.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
This is equivalent to the current behaviour, but a little clearer in its
meaning.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
The ownership transfers in this code are a bit complex, so adding some
extra documentation and `g_steal_pointer()` calls should hopefully help
clarify things.
This doesn’t introduce any functional changes, just code documentation.
Another drive-by improvement in the quest for #1264.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
The existing comment in the code was correct that `data` is freed when
the task callback is called, because `data` is also pointed to by the
`user_data` for the task, and that’s freed at the end of the callback.
So the existing code was correct to take a copy of `data->task` before
calling `g_task_return_*()`.
After calling `g_task_return_*()`, the existing code unreffed the task
(which is correct), but then didn’t clear the `data->task` pointer,
leaving `data->task` dangling. That could cause a use-after-free or a
double-unref.
Avoid that risk by explicitly clearing `data->task` before calling
`g_task_return_*()`.
After some testing, it turns out this doesn’t actually fix any bugs, but
it’s still a good robustness improvement.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
The generated gir file marks the size parameter as "out" by default. This is wrong in the context of a caller allocated buffer with a given size. Explicitly marking the size parameter as (in) fixes the issue.
As explained in the previous commit, GNU Coreutils 9.2 changes the behaviour
of `du --bytes` to only count regular files and symlinks.
The previous commit makes the test pass with GNU Coreutils >=9.2, but the
machine running the tests may have an older version, or perhaps even a
reimplementation such as uutils. So we can't rely on the size returned by `du`
to be the consistent across systems any more.
However, the plus side of the new behaviour is that the size reported by `du`
/ `G_FILE_MEASURE_APPARENT_SIZE` is now well-defined across filesystems
(as the sum of the sizes of regular files & symlinks), so we can hardcode it.
Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/2965
Since GNU Coreutils 9.2 (commit 110bcd28386b1f47a4cd876098acb708fdcbbb25),
`du --apparent-size` (including `du --bytes`) no longer counts all kinds of
files (directories, FIFOs, etc.), but only those for which `st_size` in
`struct stat` is defined by POSIX, namely regular files and symlinks
(and also rarely supported memory objects).
This aligns the behaviour of GLib's `G_FILE_MEASURE_APPARENT_SIZE` flag
with the new GNU Coreutils `du` and correct POSIX use.
Note that this may be a breaking change for some uses.
Link: https://lists.gnu.org/archive/html/bug-coreutils/2023-03/msg00007.html
Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/2965
Warn when the boolean return isn't used, since we may not initialize
the out arguments in the FALSE case. Update all internal callers to
use the return value. They were already safe, but users outside GLib
may not be.
Previously, `-Wl,--export-dynamic` was in `Libs` key of `gmodule-2.0.pc`,
even though `-Wl` is a compiler flag, rather than a linker one.
This caused issues with API reference builds in evolution-data-server,
which passes the output of `pkg-config --libs` through `--ldflags`
argument of `gtkdoc-scan`, which are forwarded unchanged to `ld`:
ld: unrecognized option '-Wl,--export-dynamic'
Let’s move the flag to `Cflags` so that the compiler can deal with it.
https://gitlab.gnome.org/GNOME/evolution-data-server/-/issues/463
Add support for it, but don’t use it anywhere yet — this is an API
addition, but currently doesn’t cause any functional changes. It’ll be
used in the next commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
