GNOME Bugzilla – Bug 666136
Doesn't handle gconf errors gracefully
Last modified: 2014-01-16 01:43:42 UTC
This issue was previously reported at https://bugs.launchpad.net/bugs/903965 I am using meld on Ubuntu Oneiric, version 1.5.2-1ubuntu2. When I start up the program, I see a window for a brief moment, and then this traceback on the console (followed by an Apport dialog): $ meld file1 file2 /usr/lib/meld/meld/meldwindow.py:576: Warning: g_object_set_qdata: assertion `G_IS_OBJECT (object)' failed self.notebook.append_page( page.widget, nbl) Traceback (most recent call last):
+ Trace 229269
main()
app.parse_args(sys.argv[1:])
tab = self.window.open_paths(args, options.auto_compare)
tab = self.append_diff(paths, auto_compare)
return self.append_filediff(paths)
doc.set_files(files)
self.fileentry[i].prepend_history(absfile)
self.__gentry.prepend_text(text)
self.__insert_history_item(text, True)
self._save_history()
self.__gconf_client.set_list(key, gconf.VALUE_STRING, gconf_items)
If I comment out historyentry.py:116, then the window stays up, and the program is usable, albeit with default preferences. (A different traceback appears in the console, and an Apport window still shows up, but these are only an annoyance.) It would be helpful if Meld complained, but did not error out when gconf is unresponsive like this. My working environment is a bit hostile to GConf (as my home directory is served via a network filesystem that is regularly inaccessible due to authentication expiry), and while GConf needs to be made more robust, Meld should remain some measure of usability without.
Adding correct error handling to all of our gconf stuff would be time-consuming, and since I'm hoping that it will be dropped for GSettings in the near future, I have to say it's unlikely to happen. Having said that, it may be possible to use a universal GConf error handler that turns all of our GConf calls into no-ops on an error. This is clearly not ideal, but nevertheless better than crashing. Do you have any idea how I could manually trigger this kind of issue without messing up everything else using gconf on my box?
I don't see an easy way of reproducing the "no writable databases" error, but I can get a similar crash (with minimal collateral damage) by temporarily freezing gconfd with SIGSTOP: $ killall -STOP gconfd-2 $ meld file1 file2 [wait a few seconds...] Traceback (most recent call last):
+ Trace 229285
from meld.meldapp import app
app = MeldApp()
self.prefs = preferences.MeldPreferences()
super(MeldPreferences, self).__init__("/apps/meld", self.defaults)
self._gconf.add_dir(rootkey, gconf.CLIENT_PRELOAD_NONE)
$ killall -CONT gconfd-2 $ meld file1 file2 /usr/lib/meld/meld/meldwindow.py:576: Warning: g_object_set_qdata: assertion `G_IS_OBJECT (object)' failed self.notebook.append_page( page.widget, nbl) [window displays normally]
So I've spent some time looking at this, and my conclusion is that unfortunately it's not possible to handle this in a sane way. Several of the GConf error handling bits that would help aren't wrapped at all in the pygtk/gobject bindings. It looks like it might be possible to extract error information using ctypes to pull data out of the opaque GError pointer that we get from GConf's error signal, but I'm really not sure that's worth it, I'm sorry. I'll leave the bug open just in case someone else wants to take a look at it. As a workaround, you can entirely disable GConf use in Meld. In meld/ui/historyentry.py, line 207 you can change the "import gconf" line to "raise ImportError" and then do the same for the "import gconf/Preferences = GConfPreferences" lines at the bottom of meld/util/prefs.py. With those changes, Meld should fall back to a flat-file backend for storing preferences.
Kai, thank you for your investigation on this; it is much appreciated. Disabling Meld's use of GConf, in favor of a flat-file backend (something in ~/.config/, presumably?), is very interesting. I use the program in a non-GNOME environment (Xfce/Xubuntu), so GConf is a bit of an awkward presence already. It's in there because some other large program (Abiword IIRC) requires it, but still, the less it's used, the better. What if there were a persistent fallback mechanism, like if Meld can't find GConf, then write settings to a flat file (I take it this already happens); then when starting up later, if the file is present, then don't touch GConf. For those like me who have GConf but would rather use a flat file, the latter could be bootstrapped by invoking e.g. "touch ~/.config/meld.conf" (if not some option to Meld itself).
So what currently happens is in two parts, because we have two different places where GConf is imported: the preferences code and the history code. The code in prefs.py is the persistent settings you want with the automatic GConf fallback, stored in ~/.config/meld/meldrc.ini. The code in historyentry.py doesn't currently have a fallback mechanism; if it can't find GConf, you just don't get history. I actually don't mind your suggestion of checking for the flat file and using that backend if present, except that I'd add an extra requirement that the file include some trigger key (e.g., "use_rc_prefs = 1" or similar). This wouldn't be difficult, but it would be a bit of work. I'd happily review a patch implementing something like this, but I can't honestly say that I'm likely to get around to writing one any time soon.
Hope everyone had (or is still having) a great holiday. I'll have a look at the prefs code when I get a chance, and see if I can get a patch together. This is something I'm wanting, and while GSettings is in the cards that doesn't seem like it'll be an improvement in terms of the complexity/fragility I've observed with GConf. As for history... I don't really care about this, but it would probably be nice if this were stored in ~/.local/share/recently-used.xbel like all the cool kids are doing...
It would be awesome if you looked at fixing our GConf handling. However, I'd caution against spending too much time looking at the HistoryEntry code with respect to gtk.RecentManager and such. At the moment, I suspect the HistoryEntry widget won't survive the move to Gtk3 (i.e., the places we use it will be replaced with a FileChooser and we'll focus on improving that experience), and wasted effort sucks.
I have tried to use meld with Git as my diff.tool, when I do it works, but when it quits, I get this warning: /usr/lib/meld/meld/meldwindow.py:576: Warning: g_object_set_qdata: assertion `G_IS_OBJECT (object)' failed self.notebook.append_page( page.widget, nbl) This was reported here: https://bugs.launchpad.net/ubuntu/+source/meld/+bug/909765 I am using Ubuntu 11.10, and Meld 1.5.2
Bugs reported in launchpad don't help me I'm sorry. Could you please open a bug here with other relevant details, such as the GTK and pygtk versions? In future, please don't attach random comments to unrelated bugs. Just opening a new one is fine, once you've checked that it's not a duplicate.
I really appreciate Meld. I use it everyday, and it is by far my most favorite diff/merge tool. I even have it installed on my windows machine at work and it works flawlessly. I didn't mean to irritate you so I'm really sorry for adding my comment with the launchpad link, which was evidently not useful and irrelevant. I thought these were the same issues, since the tracebacks are the same, although I admit I did not hack at the problem as much as I should have. If I don't find a duplicate issue I will file a new ticket after really hacking into the issue first on my own. Thanks again for Meld!!! Meld: 1.5.2-1ubuntu2 PyGtk: 2.24.0-2 GTK+2: 2.24.6-0ubuntu5
I'm now getting this error on my windows machine as well: meldwindow.py:580: Warning: g_object_set_qdata: assertion `G_ IS_OBJECT (object)' failed self.notebook.append_page( page.widget, nbl) This started after I upgraded from PyGtk 2.22.6 to 2.24.2. Something in gobject changed? also in meld-1.5.3 this error is thrown on line 580 vs. line 576 in meld-1.5.2 OS: WindowsXP_SP3 Meld: 1.5.3 PyGtk: 2.24.2 Python 2.7.2 PyGtkSourceView: 2.10.1 PyGGObject: 2.28.3
Sorry - I didn't notice that the original traceback had the same error message as well; I was just focusing on the gconf error traceback. Adding a comment here was fair enough. Thanks for the Windows pygtk version numbers - those are useful, and has narrowed the problem down more than I previously had. When you upgraded PyGtk, did that also involve upgrading Gtk itself? Anyway, I've just opened bug 670380 with some of the details you've given, so if you have any further information, please add it there. While it's certainly irritating, I don't think that this warning has any real effect, and I strongly suspect that Meld isn't at fault.
*** Bug 691612 has been marked as a duplicate of this bug. ***
Created attachment 252781 [details] [review] Patch against meld git master Okay, I've finally put together a proposed solution for this bug. Because the GConf error occurs only when you attempt to write to it, I added a test write at the points where the module is imported. So if that fails, it either falls back to configparser, or disables history, just as if the gconf module were not available. (I couldn't find out how you're supposed to delete a GConf key, which would probably be a good thing to do here.) Additionally, in prefs.py, I added a check for the meldrc.ini file; if it's present, then gconf is not used. (Kai, you said in comment #5 that you wanted an explicit trigger key in the file for this; I wasn't sure what would be the best way of checking for that.)
Thanks for looking at this. I'd like to get this in before the next stable release, as I think it's definitely worthwhile. I haven't tested this properly yet, but I don't love setting an arbitrary testing key if we can avoid it. Could we instead just read and then set one of our keys? For example, just do something like (totally untested): test_key = '/apps/meld/window_size_x' test = gconf.client_get_default().get_int(test_key) gconf.client_get_default().set_int(test_key, test) I'm not sure whether that will trigger the errors that you're looking for, but if it does I think that would be preferable. Regarding the meldrc.ini check, if your new gconf-checking code works, then do we actually need the persistent fallback, or can we just do the gconf check every time?
(In reply to comment #15) > I haven't tested this properly yet, but I don't love setting an arbitrary > testing key if we can avoid it. Could we instead just read and then set one of > our keys? I'd be worried about potentially mangling a "real" key, especially since at this point we're checking to see if GConf is working at all. Wouldn't it be just as clean if we deleted the test key right afterwards? > I'm not sure whether that will trigger the errors that you're looking for, but > if it does I think that would be preferable. There could be some "don't write to the db if the value is the same" logic in GConf. (Which could be a problem for my approach too---better to set a random instead of a fixed value...) > Regarding the meldrc.ini check, if your new gconf-checking code works, then do > we actually need the persistent fallback, or can we just do the gconf check > every time? The problem is, if GConf starts working again later, then Meld will use a different configuration on the next run. The program would have two separate configs, using one or the other depending on GConf's behavior---a sure recipe for user confusion. IMO, if GConf flakes out once, it's likely to do so again in the future, so having Meld switch persistently to meldrc mode in that case is the most robust approach.
(In reply to comment #16) > (In reply to comment #15) > > I haven't tested this properly yet, but I don't love setting an arbitrary > > testing key if we can avoid it. Could we instead just read and then set one of > > our keys? > > I'd be worried about potentially mangling a "real" key, especially since at > this point we're checking to see if GConf is working at all. Wouldn't it be > just as clean if we deleted the test key right afterwards? Yeah, I guess? I chose the window_size_x key because if it dies then I don't really care that much. Either way, if we can't write to gconf then I wouldn't expect it to do anything to the key. > > I'm not sure whether that will trigger the errors that you're looking for, but > > if it does I think that would be preferable. > > There could be some "don't write to the db if the value is the same" logic in > GConf. (Which could be a problem for my approach too---better to set a random > instead of a fixed value...) Sure. This should be easy to test however. > > Regarding the meldrc.ini check, if your new gconf-checking code works, then do > > we actually need the persistent fallback, or can we just do the gconf check > > every time? > > The problem is, if GConf starts working again later, then Meld will use a > different configuration on the next run. The program would have two separate > configs, using one or the other depending on GConf's behavior---a sure recipe > for user confusion. Yeah this is definitely bad, but at this point some things are going to not work, so it's all trade-offs. > IMO, if GConf flakes out once, it's likely to do so again in the future, so > having Meld switch persistently to meldrc mode in that case is the most robust > approach. The situation I'm thinking of is when the user runs Meld in sudo or something similar which then can't connect to the user gconf. In this case, I'd personally *much* prefer that when the user re-ran Meld in their session we went back to the gconf backend. The ini-file backend really is intended to be a fallback, not the preferred case.
Created attachment 253305 [details] [review] Revised patch Okay, new patch. Here's what I changed: * Set the key value to os.getpid(), so that it's different on every run. (I would have used a random integer, but didn't want to have to "import random") * Call unset() to delete the test key * Instead of checking whether meldrc.ini exists, check for the presence of a "use-rc-prefs" flag file in the same directory, and skip GConf if it's there. This way, Meld reverts to GConf when available again, but there still is a way to force the use of a config file.
Looks good to me. I tested your patch a bit and I've now pushed it. Thanks! It's worth noting that for me at least, the dbus timeout is quite long, so I think many users will just assume that Meld isn't working before we get the error and fallback. I don't think there's anything we can really do about this; it's just annoying.
Sounds like the same thing I saw when I SIGSTOPped gconfd. Yeah, not much we can do about that; the client has to allow gconfd some time to respond, since the latter might be swapped out to disk or the like. (In my original error case, however, GConf responded immediately---it just wasn't able to do any disk I/O.) Thanks for getting this in :) I'm glad I won't have to worry about GConf mucking up my Meld workflow anymore. For the benefit of anyone searching the Interwebs on this topic... ------------------------------------------------------------------------------ You can force Meld to ignore GConf and store its preferences in an ordinary config file by invoking the following two commands at a shell prompt: $ mkdir -p ~/.config/meld $ touch ~/.config/meld/use-rc-prefs Start up Meld, and if you see the file ~/.config/meld/meldrc.ini, then you're all set. As long as the "use-rc-prefs" file remains in place, Meld will not use GConf for preferences.
Good idea. However, bugzilla is remarkably unfriendly to google, etc. so I've actually added this to a new wiki page at https://wiki.gnome.org/Meld/GConfWorkarounds. Cheers.
Suggested workaround doesn't work. It does find this file, but doesn't like it for some reason and still fails: 11227 python2.7 CALL stat(path=0x80f73a2e0,sb=0x7fffffffb8c0) 11227 python2.7 NAMI "/root/.config/meld/use-rc-prefs" 11227 python2.7 STRU struct stat {dev=122, ino=13120759, mode=-rw-r--r-- , nlink=1, uid=0, gid=0, rdev=0, atime=1389829647, stime=1389829647, ctime=1389829647, birthtime=1389829647, size=0, blksize=16384, blocks=0, flags=0x0 } 11227 python2.7 RET stat 0
This code prints: skip= True So I don't know ---begin--- import sys import os import glib force_ini = os.path.exists(os.path.join(glib.get_user_config_dir(), 'meld', 'use-rc-prefs')) skip_gconf = sys.platform == 'win32' or force_ini print "skip=", skip_gconf ---end--- (meld:31631): GVFS-RemoteVolumeMonitor-WARNING **: cannot connect to the session bus: org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken. Traceback (most recent call last):
+ Trace 233039
already_running, dbus_app = meld.dbus_service.setup(app)
bus = dbus.SessionBus()
mainloop=mainloop)
bus = BusConnection.__new__(subclass, bus_type, mainloop=mainloop)
bus = cls._new_for_bus(address_or_type, mainloop=mainloop)
That appears to be a problem with DBus rather than GConf, although the failure mode is similar. The latest git version of Meld appears to have done away with dbus_service.py, so you may want to give that a try and see if the error persists.