Following Emmanuele's instructions for use of introspection annotations:
https://www.bassi.io/articles/2023/02/20/bindable-api-2023/
I have audited all uses of the (closure) annotation in glib and
determined that only a handful are correct. This commit changes almost
all of our use of (closure) annotations to conform to Emmanuele's rules.
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>
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>
We may not be able to fix GTasks broken design,
but at least we should document it and not let
users stumble into this bear trap without warning.
Helps: #1346
This typically indicates a bug in the program, where a GTask has been
created, but a bug in the control flow has caused it to not return a
value.
There is one situation where it might be legitimate to finalise a GTask
without returning: if an error happens in your *_async() start function
after you’ve created a GTask, but before the async operation returns to
the main loop; and you report the error using g_task_report_*error()
rather than reporting it using the newly constructed GTask.
Another situation is where you are just using GTask as a convenient way
to move some work to another thread, without the complexity of creating
and running your own thread pool. GDBus does this with
g_dbus_interface_method_dispatch_helper(), for example.
In most other cases, it’s a bug. Emit a debug message about it, but not
a full-blown warning, as that would create noise in the legitimate
cases.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Similar to g_source_set_static_name, this avoids
strdup overhead for debug-only information in
possibly hot code paths.
We also add a macro wrapper for g_task_set_name that
uses __builtin_constant_p to decide whether to use
g_task_set_name or g_task_set_static_name.
In recent Clang we may get a build warning as per:
../gio/gtask.c: warning: implicit truncation from 'int' to a
one-bit wide bit-field changes value from 1 to -1
[-Wsingle-bit-bitfield-constant-conversion]
This is because we use gboolean (and thus a signed type) for bit-fields.
Now, this is not an issue in practice for the way we're using them, but
still better to mute such compiler warns in the right way.
This makes calls to g_signal_connect_data() and g_signal_connect_object()
with default flags more self-documenting.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Add SPDX license (but not copyright) headers to all files which follow a
certain pattern in their existing non-machine-readable header comment.
This commit was entirely generated using the command:
```
git ls-files gio/*.[ch] | xargs perl -0777 -pi -e 's/\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/\n \*\n \* SPDX-License-Identifier: LGPL-2.1-or-later\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/igs'
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
However, GLib still can’t guarantee to do rate limiting, as the type of
rate limiting which is appropriate depends on what tasks are being run,
and the GTask thread pool is shared between all tasks (of many different
types) in a process space.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2368
Set counters for the number of running tasks and
for the max. threadpool size. These are meant to
get a sense for whether G_TASK_POOL_SIZE and related
constants are still suitable for current gio and
GTask usage patterns.
And improve them externally, where not otherwise set, by setting them
from the function name passed to `g_task_set_source_tag()`, if called by
third party code.
This should make profiling and debug output from GLib more useful.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
`complete_in_idle_cb()` shows up in a lot of sysprof traces, so it’s
quite useful to include the most specific contextual information we can
in it.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
This was mostly machine generated with the following command:
```
codespell \
--builtin clear,rare,usage \
--skip './po/*' --skip './.git/*' --skip './NEWS*' \
--write-changes .
```
using the latest git version of `codespell` as per [these
instructions](https://github.com/codespell-project/codespell#user-content-updating).
Then I manually checked each change using `git add -p`, made a few
manual fixups and dropped a load of incorrect changes.
There are still some outdated or loaded terms used in GLib, mostly to do
with git branch terminology. They will need to be changed later as part
of a wider migration of git terminology.
If I’ve missed anything, please file an issue!
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Once cancelled, a GTask's callback should not only be invoked
asynchronously with respect to the creation of the task, but also with
respect to the GCancellable::cancelled handler. This is particularly
relevant in cases where the cancellation happened in the same thread
where the task is running.
Spotted by Dan Winship and Michael Catanzaro.
Closes https://gitlab.gnome.org/GNOME/glib/issues/1608
Since commit 290bb0dd, where various members of GTask were converted to
a bitfield, some of the getters:
• g_task_get_check_cancellable()
• g_task_get_return_on_cancel()
• g_task_get_completed()
have been returning truthy ints (zero or an arbitrary non-zero integer)
as boolean values, rather than the canonical boolean ints of 1 and 0.
This broke the `yield` statement in Vala, whose generated C code
compares `g_task_get_completed (…) != TRUE`. i.e. Whether the
`completed` field has a value not equal to 1.
Fix this by explicitly converting truthy ints to canonical boolean ints
in all getters.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1636
Similarly to g_source_set_name(), this sets a name on a GTask for
debugging and profiling. Importantly, this name is propagated to the
GSource for idle callbacks for the GTask, ending the glorious reign of
`[gio] complete_in_idle_cb`.
The name can be queried using g_task_get_name(). Locking is avoided by
only allowing the name to be set before the GTask is used from another
thread.
Includes tests.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
There are a lot of gbooleans in the private GTask struct, which seems a
bit wasteful. Use a bitfield to compress the struct a bit.
This reduces the size of the struct from 216 bytes to 168 bytes on my
64-bit machine.
One of the fields needs to remain separate, since it’s used from a
TRACE() macro which calls typeof() on it.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
These functions already check to see if a successful result has already
been returned; expand them to also check to see if an error has been
returned.
We can’t just check GTask.result_set, as that’s actually an indicator
for whether the GTask.result member is filled — when
g_task_propagate_*() is called, it’s cleared again. We need a new state
bit.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1525
For the purposes of debugging, it is quite useful for every GSource to
have a name set. Ensure that any GSource we construct inside GLib has a
name set. For GSources which are then returned to the caller, this name
can then be overridden with something even more useful by the caller.
Since this data is only used for debugging, avoid doing any allocations
for it; just use static strings.
https://gitlab.gnome.org/GNOME/glib/issues/1175
This will fix a few broken links in the documentation, and shut up a
load of gtk-doc warnings (but certainly not all of them).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=790015
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.
The use of past tense in g_task_had_error makes one assume that it
won't forget about any errors that might have occurred. Except, in
reality, it would.
Let's use a boolean flag to remember the error once it's been
propagated, as opposed to keeping the error around. This ensures that
the g_task_propagate_* methods continue to give invalid results when
called more than once, as mentioned in the documentation.
https://bugzilla.gnome.org/show_bug.cgi?id=764163
The new "slowly add more task threads" code doesn't fully deal with
apps that queue lots and lots of tasks which then block on tasks from
their task threads. Fix this by bringing back the "task is blocking
other task" check and making sure that such tasks get bumped to the
front of the queue.
https://bugzilla.gnome.org/show_bug.cgi?id=687223
When a task is cancelled, we want to move it to the front
of the queue - our sort function does that for us, but there
is no need to resort the entire queue here, we can just
move the one item and be done with it. This uses just-introduced
threadpool api for this purpose.
https://bugzilla.gnome.org/show_bug.cgi?id=751160
