After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779265 - gio/tests/gsettings: Segfault due to double unref with GSETTINGS_BACKEND=memory
gio/tests/gsettings: Segfault due to double unref with GSETTINGS_BACKEND=memory
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 773794 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-02-26 18:05 UTC by Marvin Schmidt
Modified: 2018-05-03 10:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glib-compile-schemas: Fix various memory leaks (6.80 KB, patch)
2017-02-27 10:20 UTC, Philip Withnall
committed Details | Review
gsettings: Fix memory leak on error handling path for g_settings_set() (2.21 KB, patch)
2017-02-27 10:20 UTC, Philip Withnall
committed Details | Review
gsettings: Fix a leak in GSettingsAction (872 bytes, patch)
2017-02-27 10:20 UTC, Philip Withnall
committed Details | Review
tests: Fix a double-unref in the GSettings unit tests (915 bytes, patch)
2017-02-27 10:20 UTC, Philip Withnall
committed Details | Review
tests: Fix some memory leaks in the GSettings unit tests (1.20 KB, patch)
2017-02-27 10:20 UTC, Philip Withnall
committed Details | Review

Description Marvin Schmidt 2017-02-26 18:05:06 UTC
When running

  GSETTINGS_BACKEND=memory make -C gio/tests/ TESTS=gsettings check

I get the following errors:

  ERROR: gsettings - too few tests run (expected 34, got 27)
  ERROR: gsettings - exited with status 133 (terminated by signal 5?)

I tracked this down to the `test_schema_list_keys` test case, where at the end:

  2378   g_settings_schema_unref (schema);                                                                                                                                                
  2379   g_settings_schema_source_unref (src);

src is already NULL, when the `g_settings_schema_source_unref` call is made.
Removing line 2379 fixes the problem, but I assume the root cause lies somewhere else, since this doesn't seem to occur with different backends
Comment 1 Philip Withnall 2017-02-27 10:20:22 UTC
Created attachment 346805 [details] [review]
glib-compile-schemas: Fix various memory leaks

Spotted while running `make check` under Valgrind. While it’s not
necessary to fix memory leaks in glib-compile-schemas (since it’s a
utility which runs briefly then exits), fixing them makes more
legitimate leaks in the Valgrind output more obvious, and means we can
be sure there aren’t leaks in the underlying GLib/GIO code which
glib-compile-schemas is calling.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 2 Philip Withnall 2017-02-27 10:20:28 UTC
Created attachment 346806 [details] [review]
gsettings: Fix memory leak on error handling path for g_settings_set()

On the warning/critical error handling paths for g_settings_set(), the
GVariant value was not ref-sunk, and the schema key was leaked. This
won’t affect code in production (unless it’s seriously buggy), but
eliminates some leaks from the error testing paths in the GSettings
tests.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 3 Philip Withnall 2017-02-27 10:20:34 UTC
Created attachment 346807 [details] [review]
gsettings: Fix a leak in GSettingsAction

Every GSettingsAction was leaking its schema key (a few tens of bytes).

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 4 Philip Withnall 2017-02-27 10:20:39 UTC
Created attachment 346808 [details] [review]
tests: Fix a double-unref in the GSettings unit tests

g_settings_schema_source_get_default() is (transfer none), not (transfer
full).

Spotted by Marvin Schmidt.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 5 Philip Withnall 2017-02-27 10:20:45 UTC
Created attachment 346809 [details] [review]
tests: Fix some memory leaks in the GSettings unit tests

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 6 Matthias Clasen 2017-02-28 14:54:18 UTC
Review of attachment 346805 [details] [review]:

ok
Comment 7 Matthias Clasen 2017-02-28 14:55:21 UTC
Review of attachment 346806 [details] [review]:

sure
Comment 8 Matthias Clasen 2017-02-28 14:55:54 UTC
Review of attachment 346807 [details] [review]:

oops. good catch!
Comment 9 Matthias Clasen 2017-02-28 14:55:55 UTC
Review of attachment 346807 [details] [review]:

oops. good catch!
Comment 10 Matthias Clasen 2017-02-28 14:56:16 UTC
Review of attachment 346808 [details] [review]:

ouch
Comment 11 Matthias Clasen 2017-02-28 14:57:11 UTC
Review of attachment 346809 [details] [review]:

sure
Comment 12 Philip Withnall 2017-02-28 16:11:35 UTC
Thanks for the speedy reviews. All pushed.

Attachment 346805 [details] pushed as d6d29a2 - glib-compile-schemas: Fix various memory leaks
Attachment 346806 [details] pushed as 08b6794 - gsettings: Fix memory leak on error handling path for g_settings_set()
Attachment 346807 [details] pushed as 6bfb4ce - gsettings: Fix a leak in GSettingsAction
Attachment 346808 [details] pushed as 43fbb86 - tests: Fix a double-unref in the GSettings unit tests
Attachment 346809 [details] pushed as d892cf6 - tests: Fix some memory leaks in the GSettings unit tests
Comment 13 Philip Withnall 2018-05-03 10:46:27 UTC
*** Bug 773794 has been marked as a duplicate of this bug. ***