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
Both `GPollableInputStream` and `GPollableOutputStream` are dynamic
interfaces, in that their implementation on a class may only be
functional if certain prerequisites are met at runtime. For example,
a `GConverterInputStream` is only pollable if its base stream is
pollable, and that’s determined at runtime rather than compile time.
As such, both interfaces have a `can_poll()` method. If that method
returns `FALSE`, the behaviour of all other methods on the interface is
undefined.
That was mentioned in the documentation for `can_poll()`, but not any of
the other documentation for the interfaces, which made it a bit hard to
find.
Mention it more widely.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2952
This should split the code up into logical blocks a bit better, and make
it a bit easier to see what the test is doing at a glance.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This makes the exit conditions for each main loop clearer, and
eliminates use of global variables. It introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If the first part of the test takes less than 3s (which is normal), the
timeout for it is not removed, and could spuriously fire during the
second part of the test, causing a false failure.
Instead of relying on source IDs, just use (and explicitly destroy) a
`GSource` for the timeouts.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The test thought that calling `g_app_info_get()` was a bit of a hack,
but actually it (or calling another `g_app_info_*()` function) is the
right way to use the `GAppInfoMonitor` API.
See the documentation improvements a couple of commits back for details.
The remaining FIXME higher up in the test should probably be fixed by
getting `g_app_info_monitor_get()` to arm the signal. That requires
changes in `g_app_info_monitor_get()` to call `desktop_file_dir_init()`.
That will have to happen another time.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #799
This is put together through git archaeology:
```
git log gio/tests/appmonitor.c
```
The following commits were too trivial to have meaningful copyright:
- 54047080e9
- 4e7d22e268
- f2c1cfe8c7
- f8f344923e
- 3ce00b29ec
- 3468369625
- e9d9edde82
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
This partially reverts ed8e86a7d4.
The change to add the criticals (commit ed8e86a7d4) is correct, but
landed too late in the cycle. Let’s downgrade the criticals to debugs
for now, to stop applications seeing a lot of new criticals in their
output. Those criticals are particularly disruptive for command line
applications and unit tests.
Early in the next cycle, the debugs will be re-upgraded to criticals.
This will give applications a whole additional cycle to fix their
ambiguous use of API.
It turned out that a lot of applications have latent bugs around
calling `g_file_info_get_*()` without checking whether an attribute
is set first, and were hence relying on the ‘unknown’ return value
also being an appropriate default for them.
This was compounded by the fact that several non-local GVFS backends
were not setting `GFileInfo` attributes all the time, which caused the
‘missing attribute’ code path to be hit more frequently. For example,
they would only call `g_file_info_set_is_hidden()` with a true value and
never bother with a false one.
It was further compounded by the fact that, while this change landed for
the 2.75.4 release, there did not seem to be extensive integration
testing of that release, and distributions and downstreams went straight
to 2.76.0. That meant we missed the window between 2.75.4 and 2.76.0 to
change, fix or revert this behaviour. GLib relies on distros and
downstreams doing integration testing of unstable releases. We test with
downstream GNOME as part of gnome-build-meta, but do not have the
resources to do integration testing for everybody.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
See: #2907
See: #2932
See: #2934
See: #2945
See: #2948
This is written in pseudocode C which omits all the callback boilerplate
for the async calls. This should hopefully make the overall structure of
the loop more obvious.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #352
As suggested on #352 by Owen Taylor (commit put together by Philip
Withnall, but in Owen’s name as it’s his wording).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #352
Before commit ed8e86a7d4, this function would have silently returned a
zero-valued `GTimeVal` if the correct attributes weren’t present.
That partially regressed in commit ed8e86a7d4, which made it return with
a critical warning, but without zeroing the `GTimeVal`. The critical
warning can be ignored by users (it doesn’t abort the process unless
`G_DEBUG=fatal-criticals` is set), but the change in behaviour of
zeroing the `GTimeVal` could cause bugs.
See: #2907
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
`gio info` currently prints ‘display name:’ twice. One of those should
be ‘edit name:’ — this regressed in commit
a374b7c806.
Various translations still have the old string, but some unfortunately
do not, so this is effectively a new translatable string again.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This fixes bug #624696.
Incorporates a more recent change from 1dc774a653 to drop the
`g_type_init()` call, and reformats the indentation a bit.
Fixes: #322
They just listed built files. Since the move to Meson, these are all
kept in a separate build directory, not the source tree, so don’t need
to be ignored.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
While we can’t check for any events on it, this at least tests that
creating a file monitor works. It should cover the fix from the previous
commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: !3241
This should catch regressions in the critical warning fixed in the
previous commit.
The launch has to have several conditions:
- Session bus is running (to avoid the launch happening via the spawn
codepath)
- Use a non-existent D-Bus name (to trigger a launch error)
- Use a launch context (to hit the critical warning code path)
- Not have a startup ID specified in the platform data — this implies
having an empty launch context
- Use an async launch, as that provides an error handling path
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
It’s possible for the startup ID to be `NULL` if one wasn’t provided in
the platform data passed to `launch_uris_with_dbus()`.
Passing `NULL` to `g_app_launch_context_launch_failed()` causes a
critical warning.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The timeout runs for the entire duration of the test, which is a
function that Meson’s test harness already provides for us.
Meson’s timeout can be easily adjusted by a factor to allow for running
tests more slowly under valgrind. The timeout in the code cannot, which
leads to spurious failures like
https://gitlab.gnome.org/GNOME/glib/-/jobs/2645271.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If the timeout callback was executed, it would remove the timeout
source, leaving the `g_source_remove()` call in the main function with a
dangling source ID.
Fixes commit 73205b8bbd.
Spotted in https://gitlab.gnome.org/GNOME/glib/-/jobs/2645271.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Previously it was only being set on non-Windows platforms. For
consistency, always set it on Windows too.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2934
Following on from #2907, set various boolean attributes if they have
been requested, or are known for sure, and their value is `FALSE`.
Previously the `FALSE` value would have been implicitly returned by the
getter function, but now doing that without the attribute being
explicitly set will trigger a critical warning.
*Don’t* set these attributes if their value is unknown or there was an
error querying it.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2934
The `is-backup` attribute isn't currently set on Windows. It would
be nice to set such basic attributes on all platforms. Let's set
the attribute to `FALSE` there.
