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 740413 - Fix the GSettings Registry Backend
Fix the GSettings Registry Backend
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gsettings
unspecified
Other Windows
: Normal normal
: ---
Assigned To: Allison Karlitskaya (desrt)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-11-20 10:17 UTC by Fan, Chun-wei
Modified: 2014-11-20 14:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix the GSettings Registry Backend By Initializing cache_lock earlier (2.71 KB, patch)
2014-11-20 10:35 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2014-11-20 10:17:05 UTC
Hi,

In commit 8ff5668, GSettings was updated to delay backend subscription, which actually caused issues in the GSettings registry backend (i.e. the GSettings backend used on Windows), as there is a cache_lock that is normally used to query or update items from the registry, for example, that is only initialized during subscription.

This, as a result, resulted in access violations in programs that subsequently used g_settings_read_from_backend() without the GSettings object being subscribed, as EnterCriticalSection() is called on a NULL/invalid critical section pointer.

I will post my proposed fix for it, which is to initialize cache_lock as we initialize the registry backend, and deleting it when we finalize (I hope I am reading the code right for this).

p.s. I did see in bug 733791 that perhaps another approach would be used to fix that bug, so I would not be that sad if the patch gets outdated (rejected) in that regard :)

With blessings, thank you!
Comment 1 Fan, Chun-wei 2014-11-20 10:35:38 UTC
Created attachment 291075 [details] [review]
Fix the GSettings Registry Backend By Initializing cache_lock earlier

Hi,

Here goes the patch.

With blessings, thank you!
Comment 2 Allison Karlitskaya (desrt) 2014-11-20 13:20:38 UTC
Review of attachment 291075 [details] [review]:

This is an improvement even if we end up removing the other patch.  Thanks!
Comment 3 Fan, Chun-wei 2014-11-20 14:13:54 UTC
Review of attachment 291075 [details] [review]:

Hello Ryan,

Thanks for the fast review.  The patch was pushed as f6bbd19.

I will close this bug report shortly.

With blessings.