GNOME Bugzilla – Bug 342578
Procedure Browser (PDB) crashes after refreshing Script-Fu
Last modified: 2006-05-24 10:55:00 UTC
In current CVS, refreshing the Script-Fu scripts causes the PDB browser to crash the next time it is invoked. If the PDB browser is already open, then it may still work for a while but sooner or later it will crash due to memory corruption (with a gimp error dialog containing corrupted strings with lots of random characters). This makes it very hard to develop anything in Script-Fu while looking up stuff in the PDB, because each change to the script requires re-starting the GIMP. Interestingly, this problem does not seem to occur with Tiny-Fu. I can refresh scripts as much as I want and the PDB browser is still working fine. Steps to reproduce: - start GIMP - Xtns -> Script-Fu -> Refresh Scripts - Xtns -> Procedure Browser The plug-in crashes: glibc reports an invalid pointer passed to free().
I can't reproduce the problem and valgrind doesn't seem to see any errors. Perhaps we need a more detailed description on how to reproduce this.
OK, it looks like the problem occurs only after installing Tiny-Fu. I try to start from a clean $PREFIX and I did a "make clean && make && make install" in gimp (CVS HEAD) and gimp-tiny-fu (CVS HEAD). If I only have gimp installed in $PREFIX, everything seems to work fine, no matter how often I refresh the scripts. But after re-installing Tiny-Fu, I start seeing the crashes in the procedure browser soon after refreshing Script-Fu. It does not occur the first time I run gimp, but it occurs afterwards.
I attached to the procedure_browser plug-in using gdb (and GIMP_PLUGIN_DEBUG) and I got the following stack trace: Program received signal SIGABRT, Aborted. 0xb76c17c7 in raise () from /lib/tls/libc.so.6 (gdb) bt
+ Trace 68402
Fixed in CVS: 2006-05-24 Michael Natterer <mitch@gimp.org> * app/pdb/gimppdb.c (gimp_pdb_real_register_procedure) (gimp_pdb_real_unregister_procedure): use g_hash_table_replace() instead of g_hash_table_insert() and make sure the used key is always the name of the first procedure in the list. Fixes bug #342578. (It's actually a miracle that only the PDB browser crashed, and not GIMP, since we were using pointers to g_free()'d memory as keys when different plug-ins registered procedures with the same name)
Since 2.2 seems to have a very similar problem, perhaps it should be fixed there as well?
2.2 unregisters the procedure by *name* and always removes the first element of the list of procedures which have the same name, so e.g. script-fu will happily remove a tiny-fu procedure, or vice versa, depending on their file system order and depending on where you click refresh. It's so broken, we should just ignore it.
OK. If it can't easily be fixed, we should better not touch that code in the stable branch.
Just in case anybody looks at this bug and does not understand what was going on, here is a summary of the findings after the crazy debugging night that I had with mitch: - One of the Script-Fu scripts (web-browser.scm) is registering several procedures in the PDB without using the script-fu-* namespace. - This script was translated to Tiny-Fu (web-browser.sct) but the procedure names were not changed since they did not contain script-fu-* (which would have been mapped to tiny-fu-*). As a result, any user who had both Script-Fu and Tiny-Fu installed got some procedures registered twice. This is a minor thing that should not have caused any crash. - Unfortunately, the code in app/pdb/gimppdb.c did not handle this case correctly. As mitch mentioned in comment #4, some pointers used for keys in the hash table were freed but still used. After refreshing the list of scripts for Script-Fu, the hash table was corrupted and some keys (the ones associated with the procedures registered twice) contained random garbage. - The main process was apparently not affected much, but the procedure broswer plug-in crashed when it tried to use the invalid keys. It may have been possible to crash the main application when trying to access the procedures registered by web-browser.scm, but I did not test that.