In g_dataset_id_remove_no_notify() and g_dataset_id_get_data(), check
first the argument before taking a lock.
Note that this doesn't matter much for g_dataset_id_get_data(). We could
leave the check as it was or drop it altogether. That is because
g_dataset_id_get_data() calls g_datalist_id_get_data(), which is fine
with a zero key_id. Also, passing a zero key is not something that would
somebody to normally, so the check is not necessary at all. I leave the
check for consistency with g_dataset_id_remove_no_notify() and do the
change for the same reason as there.
However, the check for key matters for g_dataset_id_remove_no_notify().
That function calls g_data_set_internal(), which must not be called with
zero key.
At this point, it seems a code smell to perform the check after taking
the lock. Something you notice every time that you look at it, and while
technically no problem, it is ugly. Hence the change.
This is internal API, and is only used via GLIB_PRIVATE_CALL(). Such
private API is not stable. That is, you must use the same version of
e.g. libgobject and libglib.
In that case, the "Since" annotation makes no sense.
Fixes the following issues:
1. g_getenv didn't properly differentiate between non-exisiting variables
and variables with empty values. This happened bacause we didn't call
SetLastError (0) before GetEnvironmentVariable (as with most APIs,
GetEnvironmentVariable doesn't set the error code to 0 on success).
2. g_getenv called GetEnvironmentVariable, g_free, and GetLastError in sequence;
the call to g_free could change the last error code.
3. We can use the looping pattern to call APIs that return the required buffer
size. The looping pattern makes the two phases (get size and retrieve value)
appear as just one call. It's also important for values that can change at any
time like environment variables [1].
[1] https://devblogs.microsoft.com/oldnewthing/20220302-00/?p=106303
Some apps names or keywords contain multiple words. For example 'LibreOffice
Calc' contains the word 'Calc'. This is rightfully detected as a prefix match,
however generally it is expected that searching for 'calc' would consistantly
return 'Calculator' in first position, instead of ranking them equally.
We now prioritise tokens that would otherwise rank equal based on where they
occur in the string, giving earlier occurences precedence.
Definitions in definition lists in most markdown implementations,
including the GitLab one, support laziness for the definition text
(https://spec.commonmark.org/0.31.2/#lazy-continuation-line).
As a result, each defined term would be collapsed into preceding definition.
To fix this, definitions need to be separated by blank line.
The use case for exposing this field is GTK wanting reproducible
encoding output across different OSes.
I decided to expose the OS as an integer because zlib uses an int
in its header and does not make its OS codes available as a custom
type in its API.
I also decided to limit it to values between 0 and 255 because zlib
encodes the OS as a single byte.
Test included.
Fixes: #3663
Like the GWeakRef's in GObject, there is a global lock that is consulted
whenever g_main_context_unref() or g_source_destroy() or
g_source_unref() is called to retrieve a reference to the associated
GMainContext.
There are a number of actual races that are fixed by this change.
1. Racing GSource destruction with g_main_context_unref() is solved
by holding the global source_weak_locations lock while setting
source->context = NULL and while g_source_destroy() attempts to
retrieve source->context;
2. Same race as 1. but inside g_source_unref()
3. Theoretical race of double freeing the contents of
context->pending_dispatches if both g_source_destroy() and
g_main_context_unref() both free resources inside g_main_context_unref().
A couple of implementation notes:
1. Unlocking source_weak_locations too early in g_main_context_unref()
(before g_source_destroy_internal() is called) may have a race of the
G_HOOK_FLAG_ACTIVE state of the source and cause a leak of the source.
This is why source_weak_locations is also held over the calls to
g_source_destroy_internal() in g_main_context_unref(). So that
either g_main_context_unref() or g_source_destroy() (but only 1) has
the chance to free the resources associated with the GMainContext.
2. g_main_context_unref() now needs to be more of a dispose()
implementation as it can be called multiple times with losing the
last ref.
Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/803
These show up in various places in docgen-based docs, including in
results for fairly common searches (such as ‘alloc’). Whilst people
should have been scared off by these being undocumented, the red
deprecated tags make it clear these shouldn't be touched.
dispose() would previously set the "handlers" pointer to NULL. But
dispose() also calls g_signal_group_gc_handlers(), which requires this
pointer to be not NULL.
This means, dispose() could not be called multiple times. Which is a
good practice to allow, because g_object_run_dispose() and object
resurrection both requires that dispose() can be called more than once
per object.
Fix that problem, by leaving the array until finalize().
Fixes: dd43471f60 ('gobject: add GSignalGroup')
We call g_object_weak_release_all() at two places.
Once right before finalize(). At this point, the object is definitely
going to be destroyed, and the user must no longer resurrect it or
subscribe new weak notifications. In that case, we really want to
notify/release all weak notifications.
However, we also call it from g_object_real_dispose(). During dispose,
the API allows the user to resurrect an object. Granted, that is
probably not something anybody should do, but GObject makes a reasonable
attempt to support that.
A possible place to resurrect (and subscribe new weak notifications) is
when GObject calls g_object_real_dispose().
static void
g_object_real_dispose (GObject *object)
{
g_signal_handlers_destroy (object);
/* GWeakNotify and GClosure can call into user code */
g_object_weak_release_all (object);
closure_array_destroy_all (object);
}
But previously, g_object_weak_release_all() would continue iterating
until there are no more weak notifications left. So while the user can
take a strong reference and resurrect the object, their attempts to
register new weak notifications are thwarted.
Instead, when the loop in g_object_weak_release_all() starts, remember
the initial number of weak notifications, and don't release more than
that. Note that WeakRefStack preserves the order of entries, so by
maintaining the "remaining_to_notify" counter we know when to stop.
Note that this brings also an earlier behavior back, where we would call
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
This would take out the entire WeakRefStack at once and notify the weak
notifications registered at the time. But subsequent registrations would
not be released/notified yet.
It seems bad style, to use a naive realloc() +1 increment each time a new
element gets added. Instead, remember the allocation size and double the
buffer size on buffer grow. This way we get linear amortized runtime
complexity for buffer growth.
Well, WeakRefStack uses a flat array for tracking the entires. We anyway
need to search and memmove() the entries and are thus O(n) anyway. We do
that, because it allows for relatively simple code while being memory
efficient. Also, we do expect only a reasonably small number of weak
notifications in the first place.
I still think it makes sense to avoid the O(n) number of realloc() calls
on top of that. Note that we do this while holding the (per-object)
lock. It's one thing to do a linear search or a memmove(). It's another
to do a (more expensive) realloc().
Also, shrink the buffer during g_object_weak_unref() to get rid of
excess memory.
Also, note that the initial allocation only allocates space for the
first item. I think that makes sense, because I expect that many objects
will only get a single weak notification registered. So this allocation
should not yet have excess memory allocated.
Also, note that the "flexible" array member WeakRefStack.weak_refs has a
length of 1. Maybe we should use C99 flexible array members ([]) or the
pre-C99 workaround ([0]). Anyway. Previously, we would always allocate
space for that extra one tuple, but never use it. Fix that too.
Previously, at two places (in g_object_real_dispose() and shortly before
finalize()), we would call
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
This clears @quark_weak_notifies at once and then invokes all
notifications.
This means, if you were inside a notification callback and called
g_object_weak_unref() on the object for *another* weak-reference, then
an exception failed:
GLib-GObject-FATAL-CRITICAL: g_object_weak_unref_cb: couldn't find weak ref 0x401320(0x16b9fe0)
Granted, maybe inside a GWeakNotify you shouldn't call much of anything
on where_the_object_was. However, unregistering things (like calling
g_object_weak_unref()) should still reasonably work.
Instead, now remove each weak notification one by one and invoke it.
As we now invoke the callbacks in a loop, if a callee registers a new
callback, then that one gets unregistered right away too. Previously,
we would during g_object_real_dispose() only notify the notifications
that were present when the loop starts. This is similar to what happens
in closure_array_destroy_all(). This is a change in behavior, but it
will be fixed in a separate follow-up commit.
https://gitlab.gnome.org/GNOME/glib/-/issues/1002
g_object_weak_unref() would have done a fast-removal of the entry, which
messes up the order of the weak notifications.
During destruction of the object we emit the weak notifications. They
are emitted in the order in which they were registered (FIFO). Except,
when a g_object_weak_unref() messes up the order. Avoid that and
preserve the order.
Now, do a memmove(), which is O(n). But note that we already track weak
references in a flat array that requires a O(n) linear search. Thus,
g_object_weak_unref() was already O(n) and that didn't change. More
importantly, users are well advised to limit themselves to a reasonably
small number of weak notifications. And for small n, the linear search
and the memmove() is an efficient solution.
