Commit Graph

59 Commits

Author SHA1 Message Date
Sebastian Dröge
d3ae4b1d30 GSettingsBackend - Fix thread-safety during destruction of GSettings instances while notifications are emitted
g_settings_backend_watch() uses a weak notify for keeping track of
the target. There's an explanation why this is supposed to be safe but
that explanation is wrong.

The following could happen before:

1. We have the target stored in the watch list
2. The last reference to the target is dropped in thread A and we end up
   in g_settings_backend_watch_weak_notify() right before the mutex
3. g_settings_backend_dispatch_signal() is called from another thread B
   and gets the mutex before 2.
4. g_weak_ref_init() is called on the target from thread B, which at
   this point has a reference count of exactly one (see g_object_unref()
   where it calls the weak notifies)
5. Thread A continues at 3. and drops the last reference and destroys
   the object. Now the GWeakRef from 4. points to a destroyed object. Note
   that GWeakRefs would be cleared before the weak notifies are called
6. At some later point another thread g_weak_ref_get() is called by
   g_settings_backend_invoke_closure() and accesses an already destroyed
   object with refcount 0 from the GWeakRef created in 4. by thread B (or
   worse, already freed memory that was reused).

Solve this by actually storing a GWeakRef of the target in the watch
list and only access the target behind it via the GWeakRef API, and then
pass a strong reference to the notification dispatch code.

The weak notify is only used to remove the (potentially with empty
GWeakRef) target from the list of watches and the only place that
compares the target by pointer instead of going through the GWeakRef
API.

Fixes https://gitlab.gnome.org/GNOME/glib/issues/1870
2019-08-27 11:09:08 +03:00
Philip Withnall
caf5103d4b gsettingsbackend: Fix a minor memory leak
This never caused any problems because the default GSettingsBackend is
cached forever by GIOModule anyway.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-01-30 14:41:15 +00:00
Philip Withnall
f829b65e51 gsettings: Clarify ownership transfer in a few places
I was trying to debug some memory leaks in the gsettings test.
Eventually, it seems that actually they’re caused by the
GMemorySettingsBackend being cached by GIOModule — so this commit makes
no functional changes. It should make the code and documentation a bit
clearer though.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-01-30 14:41:15 +00:00
Philip Withnall
8e8f4e6486 docs: Fix various minor syntax errors in gtk-doc comments
This will fix a few broken links in the documentation, and shut up a
load of gtk-doc warnings (but certainly not all of them).

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

https://bugzilla.gnome.org/show_bug.cgi?id=790015
2017-11-07 14:51:12 +00:00
Cosimo Cecchi
88a39670e7 GSettingsBackend: use a GWeakRef during dispatch
Instead of a full reference, which causes problems for clients that
expect a GSettings instance to stop firing signals once they drop the
last reference.

https://bugzilla.gnome.org/show_bug.cgi?id=780861
2017-09-08 09:29:18 -07: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
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
Ryan Lortie
3f119b2fd4 GSettingsBackend: fix a nasty race condition
In the event that a GSettings object is being destroyed just as a change
signal is being delivered, the destroying thread will race with the
dconf worker thread for acquiring the lock on the GSettingsBackend.

If the signalling thread gets there first then the destroying thread
will block on the lock.  The signalling thread adds a reference to the
GSettings object that is being destroyed and releases the lock.  The
idea is that this should prevent the GSettings object from being
destroyed and thus maintain its entry in the list.  Unfortunately, the
weak reference notify function is already running and as soon as we
release the lock, the list entry is removed.

The signalling thread crashes.

This bug is indicative of a serious problem encountered in many
situations where GObject instances are touched from multiple threads.
Ideally, we will move to a place where g_object_ref() is not called at
all on the GSettings object from the dconf worker thread and instead, a
dispatch will be done without holding a reference (similar to how
GAppInfoMonitor presently works).  This would also prevent the
unfortunate case of someone dropping what they assume to be the last
reference on a GSettings object, only to have an already-pending signal
delivered once they return to the mainloop, crashing their program.

Making this change for GSettings (with multiple instances per thread,
the possibility of multiple backends and each instance being interested
in different events) is going to be extremely non-trivial, so it's not a
change that makes sense at this point in the cycle.

For now, we can do a relatively small and isolated tweak so that we
never access the list except under a lock.  We still perform the bad
pattern of acquiring a ref in a foreign thread which means that we still
risk delivering a signal to a GSettings object that the user has assumed
is dead (unless they explicitly disconnect their signal handler).  This
is a problem that we already had, however.

https://bugzilla.gnome.org/show_bug.cgi?id=710367
2014-03-14 09:46:39 -04:00
Ryan Lortie
698970f1f7 gsettingsbackend: a minor simplification
Change the order of the arguments on the (internal) keys_changed callback in
GSettingsListenerVTable.

This means that all functions in the table now fit the following signature:

  void (* f) (GObject             *target,
              GSettingsBackend    *backend,
              const gchar         *name_or_path,
              gpointer             origin_tag,
              const gchar * const *names);

allowing the possibility of arguments ignored at the end.

This allows us to simplify our dispatch-to-thread code in GSettingsBackend,
making it a bit less generic.

So far, this should be a straight refactor.

https://bugzilla.gnome.org/show_bug.cgi?id=710367
2014-03-14 09:46:39 -04:00
Matthias Clasen
0cc20b7e0b Don't use <filename> in docs
Switch to simpler markdown, `foo`.
2014-02-05 20:17:46 -05:00
Matthias Clasen
111803030d Don't use <envar> in docs
Switch to simpler markdown, `foo`.
2014-02-05 19:32:41 -05:00
Matthias Clasen
c575d24dfb Docs: Don't use the note tag
More markup avoidance.
2014-01-31 18:20:06 -05:00
Daniel Mustieles
078dbda148 Updated FSF's address 2014-01-31 14:31:55 +01:00
Matthias Clasen
7bea4bd89a Correct a doc comment 2014-01-01 17:59:21 -05:00
Ryan Lortie
84a6e651c2 GSettingsBackend: add read_user_value() API
This will get the 'user' value from the database (ie: the one that the user has
control over).

Provide a default implementation that chains to ->read().  That will work for
all of our internal backends which don't have a concept of layering or
lockdown.

The delayed backend implments "user value" by returning anything that's
in the changeset (incuding an explicit NULL) or chaining up otherwise.

We will use this for g_settings_get_user_value().

https://bugzilla.gnome.org/show_bug.cgi?id=668233
2013-10-28 10:19:49 -07:00
Emmanuele Bassi
54cc43630d Rename the generated private data getter function
As it turns out, we have examples of internal functions called
type_name_get_private() in the wild (especially among older libraries),
so we need to use a name for the per-instance private data getter
function that hopefully won't conflict with anything.
2013-06-24 15:43:04 +01:00
Emmanuele Bassi
32747def4b gio: Use the new private instance data declaration
Use the newly added macros, and remove the explicit calls to
g_type_class_add_private().

https://bugzilla.gnome.org/show_bug.cgi?id=700035
2013-06-24 14:18:01 +01:00
Robert Ancell
4143842eb4 Add missing allow-none annotations for function parameters.
Found using:
find . -name '*.c' | xargs grep 'or %NULL' | grep ' \* @' | grep -v '@error' | grep -v allow-none
2012-03-31 20:34:28 +11:00
Ryan Lortie
da386705f9 GSettings: two memory use fixes
First, correct a rather dubious case of accessing a GSettingsSchemaKey
after clearing it.  This was technically okay because only the key name
was accessed (and it is not owned by the struct) but it looks very
wrong.

Second, have g_settings_backend_write() sink the passed in GVariant*.
Not all backends get this right, and I'm starting to like the pattern of
virtual function wrappers being responsible for sinking the parameters
that they are documented as consuming.
2012-01-27 03:00:23 -05:00
Ryan Lortie
6e7da987b4 GSettingsBackend: allow floating refs from read()
Use g_variant_take_ref() to allow for backends to return floating
GVariant instances from their read() implementations.
2011-12-22 00:24:20 -05:00
Ryan Lortie
a795e563df Drop last uses of @returns: 2011-11-21 12:02:02 -05:00
Dan Winship
1481b7bca3 Add _g_io_module_get_default(), use to simplify other *_get_default()s
Add _g_io_module_get_default(), which implements the
figure-out-the-best-available-module-that-is-actually-usable logic,
and use that to simplify g_proxy_resolver_get_default(),
g_settings_backend_get_default(), g_tls_backend_get_default(), and
g_vfs_get_default().

https://bugzilla.gnome.org/show_bug.cgi?id=620932
2011-10-04 13:20:34 -04:00
Dan Winship
38d21f6d8a gsettingsbackend: remove useless check
GMemorySettingsBackend is always present, so there's no need to check
for "no backends available"

https://bugzilla.gnome.org/show_bug.cgi?id=620932
2011-10-04 13:20:28 -04:00
Ryan Lortie
806de91cd5 GSettings: port from GStaticMutex to GMutex 2011-09-21 16:06:53 -04:00
Ryan Lortie
27fbaf3712 GSettingsBackend: emit changes to correct thread
When g_settings_apply() is called on a delayed settings backend and
there is a D-Bus error when communicating with dconf-service, recent
versions of the dconf GSettingsBackend call a function in GLib that
improperly delivered the signal directly instead of using
g_main_context_invoke().

This patch fixes this function to route in the same way as the others so
that the signal is dispatched in the proper GMainContext.
2011-09-06 14:07:16 -04:00
Matthias Clasen
1b28408b8b Spelling fixes
Spelling fixes in comments and docs, provided by
Kjartan Maraas in bug 657336.
2011-08-29 14:49:32 -04:00
Colin Walters
b74e2a720a Stop using glib-genmarshal at build time
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
2011-06-20 17:24:07 -04:00
Ryan Lortie
a38c97af51 Allow NULL to be stored into GSettings trees
This is used for resets (in theory) and currently it crashes when
freeing the tree.
2011-05-07 11:45:44 +02:00
Ryan Lortie
68aef33404 GSettings: make _sync() a no-op if uninitialised
If GSettings is uninitialised then g_settings_sync() should very
obviously just return right away (rather than attempting to initialise
GSettings first).
2011-04-11 03:39:27 -04:00
Pavel Holejsovsky
f85909fb65 Add and update GI annotations in 'Settings'
'Settings' GIO group contains GSettings and GSettingsBackend.
2011-01-07 15:05:55 +01:00
Christian Persch
8d74c96b76 Make the memory and null settings backends public
Bug #636806.
2011-01-05 20:31:50 +01:00
Matthias Clasen
eed36d38d1 Various doc tweaks 2010-11-28 23:55:43 -05:00
Ryan Lortie
0bd50b39eb Bug 623400 - acquire context before dispatching
For GSettings.

Use the functionality introduced in the last commit to simplify our
notify dispatching and increase the safety of doing so (by ensuring that
the context is acquired in the current thread for the duration of the
dispatch).

This closes bugs #623400 and #629849.
2010-10-03 17:34:16 -04:00
Johan Dahlin
30132c44c1 Add a lot of missing annotations 2010-09-24 18:24:41 -03:00
Ryan Lortie
7b4cbbb7b2 Create GSettingsListenerVTable
...instead of passing a whole whack of function pointers around

This is an internal API.
2010-09-09 16:42:55 -04:00
Ryan Lortie
d5bd531d96 Cleanup the GSettingsBackend API
This is a substantial ABI break.  Rebuild dconf.
2010-07-22 18:39:50 -04:00
Ryan Lortie
5b946e0504 gio/: fully remove gioalias hacks 2010-07-07 19:53:22 -04:00
Ryan Lortie
746721293b GSettings: warn if use the 'memory' backend
but only if it was not explicitly selected.
2010-07-07 10:56:57 -04:00
Matthias Clasen
958f15013c Small documentation addition 2010-07-05 23:13:17 -04:00
Ryan Lortie
6d828dae38 GSettingsBackend: some debug code 2010-06-24 00:55:30 -04:00
Ryan Lortie
0010e86572 GSettings: remove "context" from backend too.
Note: this is a GSettingsBackend API/ABI change.
2010-06-17 15:06:33 -04:00
Ryan Lortie
6c3ae976e6 Bug 621266 - GSettings "context" clarification
Remove the concept of "context" in favour of dealing with
GSettingsBackend directly.
2010-06-17 14:05:40 -04:00
Ryan Lortie
597290d5c8 GSettings: major refactor. Add enums, range. 2010-06-16 18:17:53 -04:00
Ryan Lortie
a8b5353b14 Add g_settings_sync() and use it 2010-06-10 22:30:44 -04:00
Ryan Lortie
61f3f45cb9 add get_permission API to GSettingsBackend
implement it in the various in-tree backends

also, lots of whitespace changes to realign the vtable members
2010-06-05 00:10:05 +02:00
Ryan Lortie
84a0605342 change GSettingsBackend.list() API 2010-05-24 22:20:47 -04:00
Ryan Lortie
799e0242ae improve thread safety in GDelayedSettingsBackend
- hold a lock while accessing the tree of delayed values
  - use weak reference counts with the owner object to avoid doing
    g_object_notify on a dead object
  - dispatch the "has-unapplied" notify to the proper main context
2010-05-17 07:16:37 -04:00
Ryan Lortie
61219e2640 GSettingsBackend: make signal dispatch threadsafe
This commit fixes up a few race conditions in the GSettingsBackend, mostly with
respect to change notifications occuring at the same time as the last reference
count on a GSettings is dropped.  With GDBus feeding us our incoming signals in
a separate thread, this is something that could easily happen.
2010-05-17 07:16:37 -04:00
Ryan Lortie
984258c662 GSettings: support emitting signals in threads
The thread-default context that was in effect at the time that the
GSettings was created will be used for emitting signals on that
GSettings.
2010-05-17 07:16:37 -04:00
Ryan Lortie
644dfbd704 GSettings: fix mixed use of a pointer
- used in some places as a move-along-as-we-go pointer
  - used in other places as a pointer to the fixed base of an array

Switch all users to the first style to avoid a crasher.
2010-05-02 14:15:33 -05:00