Commit Graph

229 Commits

Author SHA1 Message Date
Philip Withnall
5a74c2f445 gdbusconnection: Clarify nullability of SignalInstance.sender
Following on from !1425.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-04-03 12:16:49 +01:00
Philip Withnall
ab285899a6 gdbusconnection: Document main context iteration for unsubscriptions
Add a note to the documentation of
`g_dbus_connection_signal_unsubscribe()`, `g_bus_unwatch_name()` and
`g_bus_unown_name()` warning about the need to continue iterating the
caller’s thread-default `GMainContext` until the
unsubscribe/unwatch/unown operation is complete.

See the previous few commits and #1515 for an idea of the insidious bugs
that can be caused by not iterating the `GMainContext` until
everything’s synchronised.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-02-24 09:03:02 +00:00
Philip Withnall
1a51681e6d gdbusconnection: Simplify some control flow
This removes an unhelpful `goto`. It introduces no functional changes.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-02-24 07:46:22 +00:00
Philip Withnall
bcee182a49 gdbusconnection: Drop an unnecessary GMainContext reference
`CallDestroyNotifyData` never uses that `GMainContext`, and holding a
ref to it could cause reference count cycles if the `GMainContext` is no
longer being iterated.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #1515
2020-02-24 07:46:22 +00:00
Philip Withnall
37b1acdf8c gdbusconnection: Document threading and refcounting for signals
This is essentially a mini writeup of #978.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 15:13:52 +00:00
Philip Withnall
4ec2175d21 gdbusconnection: Tidy up unsubscription code
This just removes a now-redundant intermediate array. This means that
the `SignalSubscriber` instances are now potentially freed a little
sooner, inside the locked segment, but they are already careful to only
call their `user_data_free_func` in the right thread. So that should not
deadlock.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 15:13:52 +00:00
Philip Withnall
130455bbb2 gdbusconnection: Fix race when emitting D-Bus signal callbacks
Instead of storing a copy of the `callback` and `user_data` from a
`SignalSubscriber` in a `SignalInstance` struct (which is the closure
for signal callback data as it’s sent from the D-Bus worker thread to
the thread which originally subscribed to a signal), store a strong
reference to the `SignalSubscriber` struct itself.

This keeps the `SignalSubscriber` alive until the emission is
complete, which ensures that the `user_data` is not freed prematurely.
It also slightly reduces the allocation size of `SignalInstance` (not
that it matters).

This is threadsafe because the fields in `SignalSubscriber` are all
immutable after construction.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 14:30:39 +00:00
Philip Withnall
bee27dd9f0 gdbusconnection: Tidy up destroy notification for signal subscriptions
Tie the destruction of the `user_data` to the destruction of the
`SignalSubscriber` struct. This is tidier, and ensures that the fields
in `SignalSubscriber` are all immutable after being set, so the
structure can safely be used across threads without locking.

It doesn’t matter which thread we call `call_destroy_notify()` in, since
it always defers calling `user_data_free_func` to the user-provided
`GMainContext`.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 14:30:39 +00:00
Philip Withnall
9b1c8d7dd5 gdbusconnection: Allocate SignalSubscriber structs individually
The `SignalSubscriber` structs contain the callback and `user_data` of each
subscriber to a signal, along with the `guint id` token held by that
subscriber to identify their subscription. There are one or more
`SignalSubscriber` structs for a given signal match rule, which is
represented as a `SignalData` struct.

Previously, the `SignalSubscriber` structs were stored in a `GArray` in
the `SignalData` struct, to reduce the number of allocations needed
when subscribing to a signal.

However, this means that a `SignalSubscriber` struct cannot have a
lifetime which exceeds the `SignalData` which contains it. In order to
fix the race in #978, one thread needs to be able to unsubscribe from a
signal (destroying the `SignalData` struct) while zero or more other
threads are in the process of calling the callbacks from a previous
emission of that signal (using the callback and `user_data` from zero or
more `SignalSubscriber` structs). Multiple threads could be calling
callbacks because callbacks are invoked in the `GMainContext` which
originally made a subscription, and GDBus supports subscribing to a
signal from multiple threads. In that case, the callbacks are dispatched
to multiple threads.

In order to allow the `SignalSubscriber` structs to outlive the
`SignalData` which contained their old match rule, store them in a
`GPtrArray` in the `SignalData` struct, and refcount them individually.

This commit in itself should make no functional changes to how GDBus
works, but will allow following commits to do so.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #978
2020-01-20 14:30:39 +00:00
Bastien Nocera
145dc5a49e gdbus: Fix runtime warning with debug enabled
With debug enabled, g_dbus_connection_call_done() will throw a
g_warning() if the call failed (on purpose or not) while trying to the
serial of a non-existant reply.

(/builds/GNOME/glib/_build/gio/tests/gdbus-connection:26921): GLib-GIO-CRITICAL **: 10:10:16.311: g_dbus_message_get_reply_serial: assertion 'G_IS_DBUS_MESSAGE (message)' failed
2019-12-11 11:42:03 +01:00
Philip Withnall
55f9c6d2f4 gatomic: Add various casts to use of g_atomic_*()s to fix warnings
When compiling GLib with `-Wsign-conversion`, we get various warnings
about the atomic calls. A lot of these were fixed by
3ad375a629, but some remain. Fix them by
adding appropriate casts at the call sites.

Note that `g_atomic_int_{and,or,xor}()` actually all operate on `guint`s
rather than `gint`s (which is what the rest of the `g_atomic_int_*()`
functions operate on). I can’t find any written reasoning for this, but
assume that it’s because signedness is irrelevant when you’re using an
integer as a bit field. It’s unfortunate that they’re named a
`g_atomic_int_*()` rather than `g_atomic_uint_*()` functions.

Tested by compiling GLib as:
```
CFLAGS=-Wsign-conversion jhbuild make -ac |& grep atomic
```

I’m not going to add `-Wsign-conversion` to the set of default warnings
for building GLib, because it mostly produces false positives throughout
the rest of GLib.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #1565
2019-09-21 10:48:23 +02:00
Дилян Палаузов
512655aa12 minor typos in the documentation (a/an) 2019-08-24 19:14:05 +00:00
Matthew Leeds
ce9ea30b1d gio: Fix minor docs mistakes 2019-07-15 16:07:18 -07:00
Matthew Leeds
1f49c5aaeb gio: Make minor docs improvements
This commit changes a comment in _g_dbus_worker_do_read_cb() to be
slightly more useful. At least in my experience debugging an
intermittent unit test failure in another project, this failure
condition occurred because although g_test_dbus_down() ensures that the
session GDBusConnection has exit-on-close set to FALSE before killing
its dbus-daemon, there was still a GDBusConnection on the system bus
which hit this failed read code path, because we had
DBUS_SYSTEM_BUS_ADDRESS set to the address of the #GTestDBus daemon, to
appease libudisks.

Also, make a few other minor improvements to the docs.
2019-06-25 11:11:33 -07:00
Christian Hergert
22ba4411cc gio: remove use of generic marshaller from GIO objects
Using the generic marshaller has drawbacks beyond performance. One such
drawback is that it breaks the stack unwinding from the Linux kernel due
to having unsufficient data to walk past ffi_call_unixt64. That means that
performance profiling by application developers looks grouped among
seemingly unrelated code paths.

While we can't fix the kernel unwinding here, we can provide proper
c_marshallers and va_marshallers for objects within Gio so that
performance profiling of applications is more reliable.

Related to GNOME/Initiatives#10
2019-06-17 16:29:09 -07:00
Cosimo Cecchi
7d02e32644 gdbusconnection: add a getter for the flags property
Right now this can only be set at construction but not read back.
That seems unnecessarily restrictive, and we'll need to read these
flags from outside of gdbusconnection.c in the next commit, so let's
just make it public.

https://gitlab.gnome.org/GNOME/glib/issues/1620
2018-12-20 00:41:13 +00:00
Philip Withnall
5cc6942eab gdbusconnection: Add missing (nullable) annotation to get_unique_name()
Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://gitlab.gnome.org/GNOME/glib/issues/1594
2018-11-15 09:42:33 +00:00
Philip Withnall
c63d37fdc2 docs: Clarify return/error behaviour of D-Bus signal subscriptions
Based on a patch by David Sommerseth, from
https://gitlab.gnome.org/GNOME/glib/merge_requests/285/.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-11-13 12:44:34 +00:00
Simon McVittie
e0a8df12ef Spelling: Fix spelling of "similarly"
Detected by Debian's Lintian tool.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-09-25 14:35:13 +01:00
Philip Withnall
034bbfd873 gdbusconnection: Drop an outdated TODO comment
Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://gitlab.gnome.org/GNOME/glib/issues/340
2018-07-10 19:16:35 +02:00
Piotr Drąg
d9fc99256e Use Unicode typography in new translatable strings
See https://developer.gnome.org/hig/stable/typography.html
2018-06-25 16:51:00 +02:00
Piotr Drąg
b974cccdaf Revert "Partially revert 10c490cdfe3ae042f747bd00f787492e2bdb7ed0"
This reverts commit 2d56c49b10.
2018-06-08 16:15:54 +02:00
Philip Withnall
29f4eacebe gdbusconnection: Fix a typo in the documentation for close_sync()
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: nobody
2018-04-19 16:36:16 +01:00
Simon McVittie
3d50691a30 g_test_dbus_down: Ensure next test does not use old connection
There's a race condition somewhere in GTestDBus that can result in
the next test being started at a time when g_bus_get() would still
return the connection that is in the process of closing. This can
be reproduced reasonably reliably by running the gapplication test
10K times in a loop.

Instead of relying on waiting for the weak reference to be released,
we can force the issue by clearing it.

Bug: https://bugzilla.gnome.org/show_bug.cgi?id=768996
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=894677
2018-04-10 11:22:41 +01:00
Philip Withnall
0664b61782 gdbusconnection: Fix error in g_dbus_connection_emit_signal() docs
It incorrectly said that an error could only be returned if the GVariant
was incorrect for the D-Bus API, but that’s not true: an error will also
be returned if you call it on a closed GDBusConnection.

Clarify that, and mention the actual error codes which are returned.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: nobody
2018-02-15 16:42:26 +00:00
Philip Withnall
c4ad10fede docs: Mention D-Bus reply types are tuples
The g_dbus_connection_call() documentation doesn’t make it clear that
the reply type is always a tuple.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: nobody
2018-01-09 15:17:02 +00:00
Michael Catanzaro
18f4583653 gdbusconnection: Fix link in documentation 2017-12-03 19:22:58 -06:00
Stefan Sauer
2812219adb docs: add missing '*' chars at start of doc-comments 2017-11-12 16:36:16 +01:00
Philip Withnall
3eacec1587 Use hash tables as sets in various places
Where we were already treating GHashTables as sets, modify them to use
the set-specific APIs g_hash_table_add() and g_hash_table_contains(), to
make that usage more obvious and less prone to being broken.

Heavily based on patches by Garrett Regier <garrettregier@gmail.com>.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=749371
2017-10-26 12:27:17 +01:00
Matthias Clasen
f39024038f Disambiguate source names
This makes debugging more pleasant.
2017-06-28 19:50:57 -04:00
Sébastien Wilmet
3bf4a720c3 gio/: LGPLv2+ -> LGPLv2.1+
Sub-directories inside gio/ already processed in a previous commit:
- fam/
- gdbus-2.0/ (which contains only codegen/)
- gvdb/
- inotify/
- tests/
- win32/
- xdgmime/

Other sub-directories inside gio/:
- completion/: no license headers
- kqueue/: not LGPL, BSD-style license

https://bugzilla.gnome.org/show_bug.cgi?id=776504
2017-05-29 19:53:34 +02:00
Lars Uebernickel
0751ccd315 gdbus: fix use-after-free
g_dbus_connection_call_internal() accesses the user data it passes to
g_dbus_connection_send_message_with_reply() after the call. That data
might be freed already in the case that the callback is called
immediately.

Fix this by removing the 'serial' field from the user data altogether
and fetch the serial from the message in the callback.

https://bugzilla.gnome.org/show_bug.cgi?id=748263
2017-05-12 14:43:22 -04:00
Philip Withnall
88ad0dab21 gdbusconnection: Add some comments about object ownership
Some annotations I made while trying to debug bug #781847. They
introduce no behavioural changes.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2017-04-28 12:06:25 +01:00
Ole André Vadla Ravnås
005dfeacba gdbus: fix false positive g_warning() in remove_filter()
The GDBus thread might be holding a ref while requesting to remove the
filter.

https://bugzilla.gnome.org/show_bug.cgi?id=779409
2017-04-08 01:13:06 -04:00
INSUN PYO
07465176da gdbus: Initialize types at async entrypoints
This isn't a comprehensive fix, but should cover a lot of cases
for GDBus.

https://bugzilla.gnome.org/show_bug.cgi?id=674885
2017-03-27 09:42:47 -04:00
Piotr Drąg
e9fe8868a7 Use single non-Unicode quotation marks in a new translatable message
Actually, Unicode changes to this file got reverted in
2d56c49b10. Also, there is
"No such interface '%s'" string already, so we avoid
breaking the string freeze.
2017-03-14 14:51:49 +01:00
Piotr Drąg
bb93f3a4aa Use consistent quotation marks in a new translatable message
Double quotation marks are used everywhere else in glib, as per
<https://developer.gnome.org/hig/stable/typography.html>.
2017-03-14 14:45:48 +01:00
Philip Withnall
ff327ba2d7 gdbusmessage: Add missing G_GNUC_PRINTF attribute
This highlighted a bug in GDBusConnection, where an interface name was
not included in a message referring to it.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=780032
2017-03-14 12:57:32 +00:00
Philip Withnall
78fba90f65 docs: Add links to D-Bus specification for D-Bus address format
In an attempt to clarify the format a little.
2017-02-08 15:06:23 +00:00
Fabrice Bellet
b1f14143e5 gdbus: make gdbusconnection ids thread-safe
To prevent a race where these global static counters can be
incremented by two threads concurrently.

https://bugzilla.gnome.org/show_bug.cgi?id=778096
2017-02-05 14:25:05 +01:00
Christian Hergert
18a33f72db introspection: use (nullable) or (optional) instead of (allow-none)
If we have an input parameter (or return value) we need to use (nullable).
However, if it is an (inout) or (out) parameter, (optional) is sufficient.

It looks like (nullable) could be used for everything according to the
Annotation documentation, but (optional) is more specific.
2016-11-22 14:14:37 -08:00
Matthias Clasen
2d56c49b10 Partially revert 10c490cdfe
This commit broke some tests, and I don't have the time
to fix up all the expected output, so I'll revert the changes
to the affected files for now.

This needs to be redone with the necessary test fixes.
2016-10-24 10:21:59 -04:00
Piotr Drąg
10c490cdfe Use Unicode in translatable strings
See https://developer.gnome.org/hig/stable/typography.html

https://bugzilla.gnome.org/show_bug.cgi?id=772221
2016-10-12 21:30:42 +02:00
Philip Withnall
3613b7a366 gio: Add source tags to various GTasks constructed in GLib
This makes them easier to identify when debugging and profiling.

This patch was somewhat less than interesting to write.

https://bugzilla.gnome.org/show_bug.cgi?id=767765
2016-06-29 15:16:52 +01:00
Allison Ryan Lortie
b54acf513d GDBusConnection: use uint for bitshifts
"1 << 31" is not well-defined, do use "1u << 31" instead.

https://bugzilla.gnome.org/show_bug.cgi?id=762748
2016-03-01 10:34:07 -05:00
Philip Withnall
ac05ad55fa gdbusconnection: Add missing (nullable) to get_peer_credentials()
Since Colin mentioned it on gir-devel-list.
2015-10-06 07:59:19 +01:00
Matthias Clasen
9acd0ddbf3 gio: Intern all signal names beforehand
This avoids pointless copying of static strings.
2015-09-12 11:13:45 -04:00
Dan Winship
7da3922d05 gdbus: fix race condition in connection filter freeing
If you called g_dbus_connection_remove_filter() on a filter while it
was running (or about to be run) in another thread, its GDestroyNotify
would be run immediately, potentially causing the filter thread to
crash.

Fix this by refcounting the filters, and using the existing mechanism
for running a GDestroyNotify in another thread in the case where the
the gdbus thread is the one that frees it.

Also, add a bit of documentation explaining this (and add a related
clarification to g_dbus_connection_signal_subscribe()).

https://bugzilla.gnome.org/show_bug.cgi?id=704568
2015-08-24 16:30:05 -04:00
Janusz Lewandowski
5d014a802a Add a g_dbus_connection_register_object_with_closures function
This is a binding-friendly version of g_dbus_connection_register_object.
Based on a patch by Martin Pitt and the code of g_bus_watch_name_with_closures.

https://bugzilla.gnome.org/show_bug.cgi?id=656325
2015-08-18 16:41:12 -04:00
Colin Walters
66bc9660c4 gdbusconnection: Don't g_printerr() when exiting
exit-on-close for a DBus connection is a completely normal thing.  On
a regular GNOME login, gdm retains the X server, but terminates the
session login bus and associated helpers like gnome-settings-dameon,
the a11y tools, etc.

I've seen several downstream reports of confusion as to what these
apparent error messages mean in the system log.  It doesn't help
that they're so obtuse.

We're also printing them to stderr, when this is not an error.

The reason this was introduced is presumably some people were confused
as to why their process exited when the system bus did.  But the
solution for that I believe is documentation, not printing stuff to
everyone's system log in normal operation.

https://bugzilla.gnome.org/show_bug.cgi?id=742386
2015-08-06 08:51:02 -04:00