The pattern here is that we acquire strong references via GWeakRef.
So you might think that g_object_weak_unref() is safe to call.
However, it is not safe, in case where another thread might run
g_object_run_dispose(). Which is clear, because GWeakRef only ensures we
hold a strong reference, but g_object_run_dispose() is anyway run on an
object where we already hold a reference.
In weak_unbind(), we obtain strong references, but (after) that
point, another thead might call g_object_run_dispose(). Then,
inside unbind_internal_locked() we do:
g_object_weak_unref (source, weak_unbind, context);
binding_context_unref (context);
Note that here weak_unbind might have already be unrefed, and
g_object_weak_unref() fails an assertion. But worse, the
weak_unbind callback will also be called and we issue two
binding_context_unref() and crash.
This is fixed by using g_object_weak_ref_full() (which handles the case
that the weak notification might have already be emitted) and a separate
GDestroyNotify (that is guaranteed to run exactly once).
This still doesn't make it fully work. Note that we also call
g_signal_handler_disconnect (source, binding->source_notify);
this has exactly the same problem. A concurrent g_object_run_dispose()
will already disconnect all signal handlers, and calling disconnect
fails an assertion. I think the solution for that is a new API
g_signal_handler_try_disconnect(), which does not assert. After all, the
gulong signal ID is unique (the gulong is large enough to never wrap and
there is even a g_error() check against that).
Add API g_object_weak_ref_full() and g_object_weak_unref_full().
The documentation elaborates how g_object_weak_ref() cannot be used
(naively) in a thread-safe way, when another thread is invoking dispose
(e.g. by running g_object_run_dispose() or by unrefing the last
reference). The suggestion is to combine that with GWeakRef.
Note that usually you cannot use GWeakRef alone, since you get no
notification when the reference count drop to zero. So a complete
solution uses g_object_weak_ref() and GWeakRef together.
The problem is that the user must not call g_object_weak_unref() when
another thread might just dispose() the instance. Presumably, they use
GWeakRef to first obtain a strong reference. That ensures that no other
thread can trigger dispose via a g_object_unref(). That however does not
help against another thread calling g_object_run_dispose().
In fact, there is no way to safely call g_object_weak_unref() when
another thread might call g_object_run_dispose() (unless the user
coordinates/synchronizes that at a higher level). The new API can avoid
that problem.
Note that calling g_object_weak_unref() asserts that it finds the
notification. Which might be violated by another thread calling
g_object_run_dispose() at the wrong time. But the assertion is not the
only problem. Often, the weak notification might hold resources (e.g.
an allocated data) that needs to be released -- either by the weak
notification callback or during g_object_weak_unref().
For example, see "gobject/gbinding.c". It calls `g_object_weak_unref
(source, weak_unbind, context);` while holding a strong reference on
source. But if another thread runs g_object_run_dispose() at the wrong
moment, we will call binding_context_unref() twice (and crash).
You might think, we cannot possibly support that another thread calls
g_object_run_dispose() while we still want to do something on an object.
Maybe in many cases that is not supportable. However, the user could
have a thread-safe GObject that handles that correctly. Also, see for
example GBinding, which operates on the basics of GObject (weak
notifications, GWeakRef, property notification subscription). Those
parts all all supposed to be thred-safe, and a low-level object like
GBinding should also work with objects that can be disposed by other
threads (including via g_object_run_dispose()). In general, while it's
often difficult to support multi threadded g_object_run_dispose(), the
basic g_object_weak_ref() should not fail to support such pattern.
Add new API that solves this. g_object_weak_ref_full() takes a
GDestroyNotify, which is guaranteed to be called exactly once -- unless
the data is stolen during g_object_weak_unref_full(). This can be
convenient for cleanup. But more importantly, doing the cleanup once is
also a place to synchronize and ensure we do something on the object,
while it is still alive.
We may combine g_object_weak_ref_full() with GWeakRef for a thread-safe
solution, like GBinding does. But you can now also have a fully
thread-safe solution without using GWeakRef. The benefit of that is that
you never need to acquire a strong reference on the object and never
emit a toggle notification, while still knowing the object is alive. The
unit test shows how that can be done.
The downside is that the size of WeakRefTuple increases by one pointer
size. Optimizing that would be cumbersome and is not done.
It feels ugly to leave the buffer not sized right.
We call g_object_weak_release_all() during g_object_real_dispose() and
right before finalize. In most cases, we expect that the loop iterates
until there are no weak notifications left (in which case the entire
WeakRefStack is freed). In that case, there is no need to shrink the
buffer, because it's going to be released soon anyway.
Note that no new weak references can be registered after finalize (as
the ref count already dropped to zero). However, new weak referenes can
be registered during dispose (either during the last g_object_unref() or
during g_object_run_dispose()).
In that case, I feel it is nice to bring the buffer size right again. We
don't know how long the object will continue to live afterwards, so
let's trim the extra allocation.
Refactor the function to separate the search and removal logic. Instead
of nesting the removal inside the loop, first search for the matching
entry. If none is found, return early. Otherwise, goto the removal
logic.
This reduces indentation, emphasizes the main path, and improves
readability and maintainability. The change uses the often unfairly
maligned goto for clarity.
g-ir-scanner won't pick Since or Deprecated annotations if they are
inlined, they need a dedicated documentation block for this to work. The
since annotation is used, e.g. in gtk-rs, to not expose enum flags if
not compiled declaring we have a new enough glib version.
Verify that you can delete the file from the trash before moving it, if
the file is a directory owned by the user, recursively check for
non-empty directory not owned by he user.
Closes https://gitlab.gnome.org/GNOME/glib/-/issues/1665
There is a bunch of documentation in a separate markdown page that does
not appear in search results. We should point to it.
I think the restriction on not being used to process untrusted input
should be relaxed into something more permissive, like "should not be
used on untrusted input if you care about denial of service." But that
can be a problem for another day.
There are four `Unix.+` classes in `Gio-2.0.gir` which need to be
exposed in the `Gio-2.0.gir` docs because they are actually now
cross-platform (which is a move which has caused a lot of pain).
Change the code which filters out the rest of the `Unix.+` classes to
ignore these ones. The rest of the classes continue to be documented via
`GioUnix-2.0.gir`.
Changing the regexs for this involved a fun use of negative lookahead.
See https://gitlab.gnome.org/GNOME/glib/-/issues/3697#note_2459405
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3697
After commit 34b7992fd6 the overflow check
was only done when expanding the string, but we need to do it before
checking whether to expand the string, otherwise that calculation could
overflow and falsely decide that the string is big enough already.
As a concrete example, consider a `GString` which has:
* `.len = G_MAXSIZE / 2 + 1`
* `.allocated_len = G_MAXSIZE / 2 + 1`
and `g_string_append()` is called on it with an input string of length
`G_MAXSIZE / 2`.
This results in a call `g_string_maybe_expand (string, G_MAXSIZE / 2)`,
which calculates `string->len + len` as `(G_MAXSIZE / 2 + 1) +
(G_MAXSIZE / 2)` which evaluates to `1` as it overflows. This is not
greater than `string->allocated_len` (which is `G_MAXSIZE / 2 + 1`), so
`g_string_expand()` is *not* called, and `g_string_maybe_expand()`
returns successfully. The caller then assumes that there’s enough space
in the buffer, and happily continues to cause a buffer overflow.
It’s unlikely anyone could hit this in practice because it requires
ludicrously big strings and `GString` allocations, which likely would
have been blocked by other code, but if we’re going to have the overflow
checks in `GString` then they should be effective.
Spotted by code inspection.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This works around a Meson bug
(https://github.com/mesonbuild/meson/issues/4668).
If we have a Python test which spawns a built native binary, that binary is
listed in the `depends` argument of the `test()`. On Linux, this results in
the directories containing the built libraries which the binary depends on
being added to the `LD_LIBRARY_PATH` of the test invocation. On Windows,
however, Meson currently doesn’t add those directories to `PATH` (which is
the equivalent of `LD_LIBRARY_PATH`), so we have to do it manually.
This takes the same approach as Christoph Reiter did in
gobject-introspection
(13e8c7ff80/tests/meson.build (L2)).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Unlike the previous commit, there is no `g_fdopen()` wrapper where we
can add cross-platform support for this.
I’m not adding that now just for `O_CLOEXEC` support for these two
calls, so pass the flag locally for now.
If someone wanted to add a `g_fdopen()` wrapper in future, the
`GLIB_FD_CLOEXEC` here could be refactored easily.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This adds cross-platform support for it: on glibc, musl and BSD’s libc,
the flag is natively supported. On Windows, convert it to the `N` flag,
which similarly indicates that an open file shouldn’t be inherited by
child processes.
This allows us to unconditionally pass `e` to `g_fopen()` so `O_CLOEXEC`
can easily be set on its FDs.
Also do the same for `g_freopen()`, since it shares the same underlying
mode handling code.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
While the test is structured so that each member of these arrays is only
ever accessed by one worker thread, there’s currently no barriers to
synchronise the final read (in the main thread) with the final write (in
a worker thread), which could lead to very occasional failures like this
one (https://gitlab.gnome.org/GNOME/glib/-/jobs/5119324):
```
GLib:ERROR:../glib/tests/asyncqueue.c:203:test_async_queue_threads: assertion failed (sums[i] > 0): (0 > 0)
not ok /asyncqueue/threads - GLib:ERROR:../glib/tests/asyncqueue.c:203:test_async_queue_threads: assertion failed (sums[i] > 0): (0 > 0)
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
While working on
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4648
I realized there is no assertion checking the data argument in either
g_async_queue_push_sorted() or g_async_queue_push_sorted_unlocked().
All the other push functions assert that data is not null, and the
argument is even marked as not nullable in g_async_queue_push_sorted(),
so this is clearly an oversight.
This patch adds the assert in the _unlocked() version, so that it
applies to both.
I have consciously separated this change from the other merge request,
since that one only makes documentation changes, whereas adding an
assert is a subtle change in behavior that could potentially show as
a regression in buggy applications.
Fixes https://gitlab.gnome.org/GNOME/glib/-/issues/3698
This patch marks the data arguments of the push() methods and the return
values of the pop() methods as transfer full.
This patch also marks the return values of the non-timed pop() functions
as non-nullable, as they can only return NULL when assertions fail.
In the process I also found g_async_queue_push_sorted_unlocked() was
missing a non-nullable annotation, so I added it. Given that
g_async_queue_push_sorted() has a non-nullable annotation, it makes no
sense for the unlocked version to lack it.
This is needed to avoid situations where you cannot coordinate between
bindings and libraries which also need to interact with introspection
which could affect bindings.
For example, a Python application using libpeas to load plugins, also in
Python.
Fixes: #3664
