Normally we don't really have emission hooks around, so try to allocate
only tiny array to contain a few of them and in case we exceed that limit,
we go back to use allocated ones.
We used to call this function as unlocked, with a node value that
could be invalid at the point of the call, so let's ensure that when
we call such function it's defined, and then reduce the access to the
signal node members when we're unlocked or after a lock/unlock operation
that may have changed it.
As per this, add more tests handling multiple signal hooks cases that we
did not cover before.
In g_signal_emit_valist() we used to access to param types array and
n_params values after unlocking the mutex, and this might have lead to
making such values unreliable for the current call.
So let's keep them around until we're done with the function call
Since we're locking and unlocking once we've found the signal ID, we
might have performed calls to g_signal_emit_valist() with a signal id
that was already been removed, and thus failing later.
This is not really an issue as inside g_signal_emit_valist() we were
re-checking for the signal id, but we can make this more reliable so
that the first thread that acquires the lock can also be sure to emit.
Calling g_signal_handlers_block/unblock/disconnect_matched with only G_SIGNAL_MATCH_ID
do not match any handlers and return 0.
Fixes: #2980
Signed-off-by: Przemyslaw Gorszkowski <pgorszkowski@igalia.com>
The use of ‘OR’ in the existing documentation suggests that the matching
is disjunctive, but it’s actually conjunctive. Clarify that in the
documentation and add a test.
Spotted while reviewing
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3376.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
- Insert missing word "from".
- Remove space between function name and "()" so syntax highlighting
can recognise it as a function.
- Avoid "you"/"your" when discussing the reentrancy issues of regular
UNIX signal handlers, because it gives the false impression that
these issues are applicable to g_unix_signal_source_new().
Unrelated:
- Fix missing space in documentation of g_signal_new_class_handler().
All of these warnings indicate programmer error, so critical is most
appropriate here.
Exceptions: deprecation warnings are just warnings. Also, warnings that
are worded with uncertainty can remain warnings rather than criticals.
In g_signal_parse_name we were looking up for the signal from the name
keeping the mutex locked, but we then retrieved and checked the node
data without keeping the lock, so with another thread potentially
changing that.
We used to perform unneeded lock/unlock dances to perform block, unblock
and disconnect actions, and these were potentially unsafe because we
might have looped in data that could be potentially be changed by other
threads.
We could have also done the same by saving the handlers ids in a
temporary array and eventually remove them, but I don't see a reason for
that since we can just keep all locked without the risk of creating
deadlocks.
Coverity CID: #1474757, #1474771, #1474429
As with commit 0932f71460, which did this for refs/unrefs of the
object in `g_object_notify()`, we need to do a similar thing for
refs/unrefs of the instance with `g_signal_emit()`, for all the same
reasons.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
We currently keep a flag for whether an object has
ever had any signal handlers. But even if it had signal
handlers, it may not have any notify handlers. Keep that
information separately, so we can speed up property setting.
These have all been added manually, as I’ve finished all the files which
I can automatically detect.
All the license headers in this commit are for LGPL-2.1-or-later, and
all have been double-checked against the license paragraph in the file
header.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
Convert all the call sites which use `g_memdup()`’s length argument
trivially (for example, by passing a `sizeof()`), so that they use
`g_memdup2()` instead.
In almost all of these cases the use of `g_memdup()` would not have
caused problems, but it will soon be deprecated, so best port away from
it.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319
We have a "good" implementation of g_clear_signal_handler() in
form of a macro. Use it, and don't duplicate the code.
Also add a comment to the documentation that "instance" in fact must
not point to a valid GObject instance -- if the handler ID is unset.
Also reword the documentation about the reasoning for why a macro
version exists. The reason is not to use the function "without
pointer cast". I don't think the non-macro version requires any
pointer cast, since "instance" is a void pointer. Was this referring
to the handler_id_ptr? That doesn't seem right either, because the
caller should always provide a "gulong *" pointer and nothing else.
g_signal_new_valist() is called by g_signal_new(), which is probably
the most common way to create a signal.
Also, in almost all cases is the number of signal parameters small.
Let's optimize for that by using a stack allocated buffer if we have
few parameters.
Also adds a test that checks that the G_SIGNAL_RUN flags are handled
correctly and the class signal handler is called at the right times.
Fixes https://gitlab.gnome.org/GNOME/glib/issues/513
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
commit 916297be79 added a hash table
to provide constant time lookups of signal handlers.
Unfortunately, that commit neglected to remove handlers from
g_signal_connect_object calls from the hash table that are
disconnected implicitly when the associated object goes away.
This commit addresses that bug by changing the closure invalidate
handler associated with the signal connection to properly remove the
handler from the hash table.
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>
Making this validation code public allows projects to validate a
GParamSpec name before creating it. While hard-coded GParamSpec don't
need this, we can't afford crashing the main program for dynamically
generated GParamSpec from user-created data.
In such case, we will need to validate the param names and return errors
instead of trying to create a GParamSpec with invalid names.
Includes modifications from Philip Withnall and Emmanuele Bassi to
rearrange the new function addition and split it into one function for
GParamSpecs and one for GSignals.
The `G_TYPE_IS_INSTANTIATABLE()` check is redundant with a precondition
on the function.
The `g_type_class_peek()` check seems like a pointless restriction: it
should be possible to check for a signal from a class init function.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #650
The format has never previously been specified. It can be anything, but
for sanity’s sake disallow empty strings.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #358
Rather than adding a canonicalised and non-canonicalised version of the
signal to `g_signal_key_bsa`, just add the canonicalised version. Signal
lookups always use the canonicalised key (since the previous commit).
This saves space in `g_signal_key_bsa`, which should speed up lookups;
and it saves significant space in the global `GQuark` table (a 9.6%
reduction in entries in that table, by a rough test using
gnome-software).
We have to be a little more relaxed on the signal name validation than
we are for property name validation, as GTK installs a
`-gtk-private-changed` signal which violates the signal naming rules.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Previously, we’d look up the signal name as passed to (for example)
`g_signal_lookup()`, and rely on the fact that signals are inserted
twice into `g_signal_key_bsa`; once in canonical form and once not.
In preparation for only inserting signals into `g_signal_key_bsa` once,
we now try looking up a signal with the given signal name and, if that
fails, try canonicalising the name and trying again.
This is a performance hit on lookups for non-canonical names, but
shouldn’t affect the performance of lookups for canonical names. If
people want performance, they should use canonical names.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #358
This eliminates a call from every call site of signal_id_lookup(). It
introduces no functional changes, but allows subsequent refactoring.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Since signal names are the same as property names, reference between the
two. Improve the formatting, and make it clearer that `_` is
discouraged.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #358
This uses a 32bit hole in the GObject structure on 64bit arches
as a flag field which can be optionally used for some preformance hints.
Currently there is a flag that gets set any time you connect to a signal
on a GObject which is used as early bailout for signal emissions, and using
the flags field instead of a user-data for checking if a GObject is
under construction.
