If you’re only quickly looking at the API signature, it looks like
`item_free_func` will be called for all items enqueued to the thread
pool.
As it happens, it’s actually only called for the items which are still
enqueued when the thread pool is destroyed. The user’s `GFunc` is
responsible for freeing items which are successfully dequeued and
processed during the lifetime of the thread pool.
That’s a bit of a gotcha, so document it more explicitly.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If a timeout executes on the same main context iteration as completion
or cancellation of a resolver lookup, `has_returned` will be set
multiple times. That’s fine (the `GCond` will be notified multiple
times, but that’s fine). It was triggering an incorrect assertion, so
remove that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The default for the class is still to have no timeout, but it seems more
practical for most use cases to set a non-infinite timeout on the
default resolver.
If applications have a more specific use case, they can change the
timeout or replace the default resolver.
See https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3397#note_1731387
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If `async_cancel()` was invoked, it would remove the IO watch source,
which would cause the `g_source_remove()` call at the end of `main()` to
warn about an unknown source ID.
Fix that by handling the source as a pointer instead of a handle.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Rather than running lookups in the global shared thread pool belonging
to `GTask`, run them in a private thread pool.
This is needed because the global shared thread pool is constrained to
only 14 threads. If there are 14 ongoing calls to
`g_task_run_in_thread()` from any library/code in the process, and then
one of them asks to do a DNS lookup, the lookup will block forever.
Under certain circumstances, particularly where there are a couple of
deep chains of dependent tasks running with `g_task_run_in_thread()`,
this can livelock the program.
Since `GResolver` is likely to be called as a frequent leaf call in
certain workloads, and in particular there are likely to be several
lookups requested at the same time, it makes sense to move resolver
lookups to a private thread pool.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This will make it simpler to handle timeouts and cancellation in future,
as all the logic for working out whether to return will all be in one
place, and all the lookup-specific code is now implemented in simple
sync functions which don’t need to care about `GTask`s.
This commit introduces no functional changes, it’s just setting up for
the following commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This introduces no functional changes, but will make a reorganisation of
the code simpler in the next commit.
Rather than dealing with three different closure types, this changes the
code to deal with one which is a tagged union of the three.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The class and its header are not public, so this should not be an API or
ABI break.
This just simplifies the code a little and allows for easy extension of
the object’s private data in future commits.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Without a timeout, some lookup requests can go on forever, typically due
to bugs in underlying systems.
This can have particularly significant effects on the Happy Eyeballs
algorithm in `GSocketClient`, which relies on multiple name lookups as
its first step.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2866
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 not produced anything but false positives for several years now,
and it would be better to save the CI/analysis/triage resources and
instead focus on `scan_build` reports, which generally seem to be more
useful.
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>
If both __NR_futex and __NR_futex_time64 are defined, g_futex_simple()
will first call futex_time64(). If that fails with ENOSYS, then
futex_time() is called instead. However, errno was not saved and
restored in this case, which would result in g_futex_simple()
returning with errno set to ENOSYS, even if futex_time() succeeded.
These make it a bit easier to track the ongoing resolver tasks, as the
tasks and/or their closures are not tracked in a big list somewhere.
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>
As with the previous commit.
The logic has to be a little contorted here to avoid leaving the context
locked after emitting the critical warning. Execution does (and should)
continue after a critical warning by default, so we should do our best
to recover.
Inspired by https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3302.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The documentation was fairly clear before, but we can make it clearer:
it’s a programming error to call `g_main_context_release()` if you have
not received a true return value from `g_main_context_acquire()` before.
Inspired by https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3302.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
We need a way to initialise refcounted types placed in static storage,
or on the stack. Using proper macros avoids knowing the magic constant
used for grefcount and gatomicrefcount.
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>
In non-perf mode, we were making only one thread to win the race to increase
the value, but since we're launching more threads anyways let's allow
more of them to have a chance to do something, to make the test more
valuable, even if it's still quick enough.
This removes the `comment` member of the GKeyFileGroup structure, which
seemed intended to distinguish comments just above a group from comments
above them, separated by one or more blank lines. Indeed:
* This does not seem to match any specification in the documentation,
where blank lines and lines starting with `#` are indiscriminately
considered comments. In particular, no distinction is made between the
comment above the first group and the comment at the beginning of the
file.
* This distinction was only half implemented, resulting in confusion
between comment above a group and comment at the end of the preceding
group.
Instead, the same logic is used for groups as for keys: the comment
above a group is simply the sequence of key-value pairs of the preceding
group where the key is null, starting from the bottom.
The addition of a blank line above groups when writing, involved in
bugs #104 and #2927, is kept, but:
* It is now added as a comment as soon as the group is added (since a
blank line is considered a comment), so that
`g_key_file_get_comment()` returns the correct result right away.
* It is always added if comments are not kept.
* Otherwise it is only added if the group is newly created (not present
in the original data), in order to really keep comments (existing and
not existing).
Closes: #104, #2927
