Destroying a GDBusProxy in a thread used to race with NameOwnerChanged
being delivered to the main context's thread (GNOME #651133).
Also, g_dbus_proxy_call_sync in a thread would race with NameOwnerChanged
being delivered to the main context's thread and rewriting the name_owner
(GNOME #656039).
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=656039
Bug-NB: NB#259760
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Signed-off-by: David Zeuthen <davidz@redhat.com>
This changes the meaning of "properties_lock" from "lock for D-Bus
properties" to "lock for GObject properties".
The most common problem, and the only one I've reproduced in a regression
test, is name_owner, which can be updated by the thread that owns
the GDBusProxy's main context (i.e. the thread-default main context of
the thread that constructed it) at the same time that a blocking call
is made. When a GDBusProxy is constructed in a thread-pool thread for
short-term use, the main context will typically be the global default
main context (which is actively running in the main thread!), making
this extremely problematic.
The interface info is perhaps a theoretical concern - one thread could
conceivably set it at the same time that another thread uses it, but only
in relatively pathological situations. The current API for this does have
the problem that it returns a borrowed ref, but interface info is
hopefully permanent anyway.
The default timeout is probably only a theoretical concern - it's just an
int, so writes are indivisible, and there's no worry about whether
something has been freed - but to be safe, let's hold the lock for that
too.
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=656039
Bug-NB: NB#259760
Signed-off-by: David Zeuthen <davidz@redhat.com>
This removes the need for async_init_get_name_owner_cb to cope with being
called without a real GAsyncResult, and will simplify the addition of
correct thread-locking.
In async_init_data_set_name_owner, use the name_owner parameter instead
of the corresponding member of GDBusProxyPrivate, partly to reduce
pointer-chasing but mainly to avoid needing to hold the lock.
Signed-off-by: David Zeuthen <davidz@redhat.com>
These ought to have thread-locking, and having it in the accessor seems
better than duplicating it here.
Signed-off-by: David Zeuthen <davidz@redhat.com>
If you run:
( cd gio/tests && G_DBUS_DEBUG=all ./gdbus-proxy-well-known-name )
you can see that in the case where the name com.example.TestService isn't
owned yet, the GDBusProxy calls GetAll() with no destination, resulting
in an error reply from the peer (the dbus-daemon itself). That's clearly
not right!
However, if priv->name is NULL, that indicates the special case where we
really do want to talk directly to a peer, instead of via the bus daemon
(most likely to be used on peer-to-peer connections); in that special
case, do call GetAll().
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Signed-off-by: David Zeuthen <davidz@redhat.com>
This is needed because the proxy may need to update its internal state
which a signal handler connected to the manager may rely on.
Signed-off-by: David Zeuthen <davidz@redhat.com>
This avoids calling g_variant_unref and g_free on uninitialized memory
if PropertiesChanged is received in the creating thread's thread-default
main context's thread, at the same time as releasing the last ref in
another thread. This would result in "goto out" before the variables
freed after that label had been initialized to NULL.
Based on a patch by Simon McVittie, bug 656282
Prepare for the future where udisks will use $XDG_USER_DIR/Volumes
instead of /media when mounting filesystems on behalf of the user.
Signed-off-by: David Zeuthen <davidz@redhat.com>
Found by GIR compiler when building gobject-introspection:
gir/gio-2.0.c:33525: Warning: Gio: g_tls_password_set_description: unknown
parameter 'flags' in documentation comment, should be one of 'password',
'description'
gir/gio-2.0.c:14568: Warning: Gio: g_action_group_action_state_changed: unknown
parameter 'state' in documentation comment, should be one of 'action_group',
'action_name', 'value'
The database is an abstract object implemented by the various TLS
backends, which is used by GTlsConnection to lookup certificates
and keys, as well as verify certificate chains.
Also add GTlsInteraction, which can be used to prompt the user
for a password or PIN (used with the database).
https://bugzilla.gnome.org/show_bug.cgi?id=636572
socket->priv->connected was only being set if g_socket_connect()
succeeded right away; in the case where it returns G_IO_ERROR_PENDING,
it never got set. Fix that by having g_socket_check_connect_result()
set it on success.
Otherwise, we may run into trouble as opening a peer-to-peer
connection uses a socket client, which uses a proxy resolver
which may end up using gsettings, whose dconf backend may end
up using the session bus to talk to dconfd...
Also add convenience _with_unix_fd_list variants to GDBusConnection,
GDBusProxy and GDBusMethodInvocation types to easily support this.
Signed-off-by: David Zeuthen <davidz@redhat.com>
The code-generator already uses GVariant* so generated code didn't
really work at all. We want that instead of gint to avoid confusion
because a 'h' instance is an _index_ into a GUnixFDList, not a file
descriptor.
Signed-off-by: David Zeuthen <davidz@redhat.com>
This is possible now that we have better support for object path
arrays, see
http://git.gnome.org/browse/glib/commit/?id=19878998bc386db78614f1c92ff8524a81479c7b
Note that this breaks the ABI of generated code but since
gdbus-codegen(1) has never yet been in a stable GLib release, this is
fine.
Signed-off-by: David Zeuthen <davidz@redhat.com>
- move choice of statfs vs statvfs from gio/glocalfile.c to configure.ac
- if statvfs is the choice, then don't check number of arguments to statfs()
- use choice in gio/gunixmounts.c as well
https://bugzilla.gnome.org/show_bug.cgi?id=617949
This function implements the following logic:
if (g_variant_is_floating (value))
g_variant_ref_sink (value);
which is used for consuming the return value of callbacks that may or
may not return floating references.
This patch also replaces a few instances of the above code with the new
function (GSettings, GDBus) and lifts a long-standing restriction on the
use of floating values as the return value for signal handlers by
improving g_value_take_variant().
https://bugzilla.gnome.org/show_bug.cgi?id=627974
The -e parameter to echo isn't recognized by echo in POSIX sh,
but isn't needed when no escaped characters need to be
interpreted.
This fixes building glib with a mingw cross compiler on Mac OS X.
https://bugzilla.gnome.org/show_bug.cgi?id=654085
-In gio/Makefile.am, the name for one of the filters for capturing the
sources for the GIO VS Project Files is corrected.
-Remove the GIO source file items in the VS project files templates as
a result for this change, and move the entry of the "new"
gregistrysettingsbackend.c into the filter in gio/Makefile.am
GAction is a read-only interface (as is visible by the lack of _set() functions
on its API). The properties on the interface currently force implementors to
support writing of the properties at construct time, however.
Lift that restriction.
Take advantage of this from GSimpleAction by nuking the set_property
function and setting the fields directly in the constructor.
This commit represents an API break to GAction in the following ways:
- the 'set_state' entry in the GActionInterface vtable has been
renamed to 'change_state'. The number and order of vtable items has
not otherwise changed.
- g_action_set_state() has been renamed to g_action_change_state() to
match the updated vtable entry.
- the "state" property of the GAction interface has been changed to
read-only to reflect the fact that g_action_set_state() no longer
exists.
- GSimpleActionClass has been hidden. GSimpleAction can no longer be
subclassed.
>> Rationale
g_action_set_state() has never been a true setter in the sense that
calling it will update the value of the "state" property. It has always
been closer to "request 'state' to be changed to this value" with
semantics defined by the implementor of the interface. This is why the
equivalent method in GActionGroup had its name changed from 'set' to
'change'. This change makes the two interfaces more consistent and
removes any implication about the effect that calling set_state() should
have on the 'state' property.
>> Impact
This incompatible API break was undertaken only because I strongly
suspect that it will go entirely unnoticed. If the break actually
affects anybody, then we will accommodate them (possibly going as far as
to revert this commit entirely).
The virtual table change only impacts implementors of GAction. I
strongly suspect that this is nobody (except for GSimpleAction).
The hiding of GSimpleActionClass only impacts impacts subclasses of
GSimpleAction. I strongly suspect that none of these exist.
The changing of the property to be read-only only affects people who
were trying to change the state by using GObject properties. I strongly
suspect that this is nobody at all.
The removal of the g_action_set_state() call is the most dangerous, but
I still suspect that it will impact nobody outside of GLib. If anybody
is impacted by this change then, at their request, I will reintroduce
the API as a deprecated alias for g_action_change_state().
To help cross compilation, don't use glib-genmarshal in our
build. This is easy now that we have g_cclosure_marshal_generic().
In gobject/, add gmarshal.[ch] to git (making the existing entry
points stubs).
In gio/, simply switch to using g_cclosure_marshal_generic().
https://bugzilla.gnome.org/show_bug.cgi?id=652168
... otherwise we might end up using the worker after it has been
freed. Reported by Dan Winship and Colin Walters.
This fix uncovered a bug in the /gdbus/nonce-tcp test case so "fix"
that as well to use a better way of having one thread wait for another
(using quotes for the word "fix" since it's pretty hackish to
busy-wait in one thread to wait for another).
Signed-off-by: David Zeuthen <davidz@redhat.com>
These will validate the resulting line, and throw a conversion error.
In practice these will likely be used by bindings, but it's good
for even C apps too that don't want to explode if that text file
they're reading into Pango actually has invalid UTF-8.
https://bugzilla.gnome.org/show_bug.cgi?id=652758
g_data_input_stream_read_line() and
g_data_input_stream_read_line_finish() don't do any encoding checks,
so we shouldn't call the returned value a "string" (which I'd like to
mean UTF-8). Annotate them as byte arrays and add encoding warnings
to the docstrings.
https://bugzilla.gnome.org/show_bug.cgi?id=652758
Matthew Bucknall pointed out
GDBusMessage does not serialize/deserialize double values correctly
on platforms with strict alignment constraints (in my particular
case, ARM926EJ-S).
This was reported in
https://bugzilla.gnome.org/show_bug.cgi?id=652197
Signed-off-by: David Zeuthen <davidz@redhat.com>
Rather than having the gtk-doc build machinery have a list of header
files to exclude, change the GLib build to dump a list of public
header files generated from the maintained Makefile.am files for
each of glib/, gobject/, gio/.
Also, for glib, always install glib-unix.h, even on non-Unix
platforms, for the same reason we install gwin32.h even on Unix.
https://bugzilla.gnome.org/show_bug.cgi?id=651745
See https://bugs.freedesktop.org/show_bug.cgi?id=37890#c6 where it was
discovered that dbus-send(1) actually doesn't work (either libdbus-1's
flush implementation or dbus-send(1)'s usage of it is broken) so it's
useful to have here.
Signed-off-by: David Zeuthen <davidz@redhat.com>
This avoids the generated types (e.g. ExampleAnimal, ExampleCat,
ExampleObject and ExampleObjectManagerClient) being referenced in the
core gio docs. This was requested by Matthias.
Signed-off-by: David Zeuthen <davidz@redhat.com>
I can't see a reason to spin until the worker thread runs, so don't.
This avoids ugly sched_yield() calls that show up in strace and
annoy me; the code is cleaner now too.
We now grab the types needed for the WebKit workaround in the
thread creation area, but only release them when the thread itself
exits.
https://bugzilla.gnome.org/show_bug.cgi?id=651650
In resolve_sync function in gthreadedresolver.c, if g_thread_pool_push
fails due to thread creation failure, we are just simply appending the
data to the queue of work to do. After the failure, we might wait
indefinitely in g_cond_wait. In case of g_thread_pool_push failure,
propagate the error so that this function does not blocks forever in
case of failure.
https://bugzilla.gnome.org/show_bug.cgi?id=651034
The fact that we return 0 here makes it clear that this
is not considered an error, so it makes sense to not
write these messages to stderr.
Proposed by Antoine Jacoutot,
https://bugzilla.gnome.org/show_bug.cgi?id=650882
The grouping in files/headers is not used anymore, and
the function attributes neither. Adapt abicheck scripts
and .def file generation rules accordingly.
There are some bugs caused by the way that gsettings-tool currently
attempts to help the user when they leave the quotes off of a string
value that they are setting.
Simplify the code to make it more robust and add some comments about why
it should be done this way.
https://bugzilla.gnome.org/show_bug.cgi?id=649915
For example, if setting a property on a skeleton from another thread
than where it was constructed, the idle handler responsible for
emitting the PropertiesChanged() signal could run immediately and
clear skeleton->priv->changed_properties_idle_source causing
g_source_unref() to be called with a NULL pointer. This race was
easily be fixed by adding a lock to the skeleton object.
In addition to fixing this race, also move the code for setting up the
idle handler to a class handler for the GObject::notify signal. This
change allows use of g_object_freeze_notify() and g_object_thaw_notify()
to perform atomic property changes from another thread than the one
that the skeleton was created in.
Signed-off-by: David Zeuthen <davidz@redhat.com>
Don't drop resets from a GDelayedSettingsBackend when the writability
changes. Resets will always succeed, even against non-writable keys and
some people (gnome-screensaver) are using them in a way that they would
want them not to be forgotten.
This can easily happen if the owner of the remote object vanishes. Of
course, when that happens, user code is already notified (by e.g. the
notify::g-name-owner signal) so it can avoid using the proxy but
requiring that is a bit harsh. IOW, before this patch this critical
error was printed
GLib-GIO-CRITICAL **: g_dbus_gvariant_to_gvalue: assertion `value != NULL' failed
when that happened. With this patch, we just avoid setting the GValue
so the user will get the default value for its type instead. So, for
example, if the user code is getting a GVariant property on such a
defunct proxy, then he gets a NULL back. So unless said user code
checks the return value, criticals will still be printed if the NULL
GVariant is used for anything interesting.
Signed-off-by: David Zeuthen <davidz@redhat.com>
The code generator sprinkles a few asserts in its output of the form:
g_assert (prop_id - 1 >= 0 && prop_id - 1 < %d);\n
prop_id is unsigned, though, so this generates a compiler warning for
me.
This commit changes the code to merely check for prop_id != 0 instead of
prop_id - 1 >= 0
.. and add a C setter to do this. Also make the C getter return a
reference since the property may be set from another thread. Also
change the constructor to _not_ take a GDBusConnection since this is
something you almost always want to do _after_ creating it. The
API/ABI break is fine as there has never been a GLib release with this
type.
https://bugzilla.gnome.org/show_bug.cgi?id=648959
Signed-off-by: David Zeuthen <davidz@redhat.com>
_GNU_SOURCE must be defined before including any other (system)
header, so defining it in glib-unix.h (and hoping no one has included
anything else before that) is wrong. And the "#define _USE_GNU"
workaround for this problem in gnetworkingprivate.h is even wronger
(and still prone to failure anyway due to single-include guards).
Fix this by defining _GNU_SOURCE in config.h when building against
glibc. In theory this is bad because new releases of glibc may include
symbols that conflict with glib symbols, which could then cause
compile failures. However, most people only see new releases of glibc
when they upgrade their distro, at which point they also generally get
new releases of gcc, which have new warnings/errors to clean up
anyway.
https://bugzilla.gnome.org/show_bug.cgi?id=649201
GDesktopAppInfo violates the GObject rule that your C constructors
should just be thin wrappers around g_object_new(). While GKeyFile
isn't introspctable, this patch allows from JavaScript:
var app = new Gio.DesktopAppInfo({ filename: '/path/to/foo.desktop' });
https://bugzilla.gnome.org/show_bug.cgi?id=648425
When an old pre-thread-default-context API that takes an explicit
GMainContext wants to call a gio API, it must call
g_main_context_push_thread_default() before, and
g_main_context_pop_thread_default() after the gio call, so that the
gio method will return its result to the desired GMainContext.
But this fails for methods like g_socket_client_connect_async() that
make a chain of multiple async calls, since the pushed/popped context
will only affect the initial call.
Fix this by having GSimpleAsyncResult itself push/pop the context
around the callback invocation, so that if the callback queues another
async request, it will stay in the same context as the original one.
https://bugzilla.gnome.org/show_bug.cgi?id=646957
