That is not enough information to deal with this report. Please tell us exactly what operating system you are using, what version of GTK+ and where you got your GIMP and GTK+ installations from.

heh. Sorry. I should have been much better about that what with all the pidgin bugs I've filed. On Windows XP Service Pack 2. GTK 2.10.13 (rev A for Pidgin). If I remember correctly, it was an incomplete installer packaged by datallah. The GIMP installer I got from gimp-win.sourceforge.net. I guess the OS field isn't specific enough ;) Let me know if you need anything else. (btw I'm using the wimp theme right now)

Is this bug #314529 again?

Ankit, please try the GTK+ installer from gimp-win.sourceforge.net. Gaim/Pidgin is known to ship broken versions of GTK+ and you are likely experiencing a problem due to using such a broken version.

I will try when I return home this evening. Why is it that every product thinks *their* version of GTK is the *right* one to use? Is it possible to not have GTK shared between GIMP and Pidgin?

It would be possible if the Gaim/Pidgin people didn't release a broken version every so often. The bug in question has long been fixed in GTK+. Looks like what Pidgin ships is not version 2.10.13 as they claim.

This can be closed as a dupe of bug #314529. Define "long been fixed" as in what version and how? Would I just need to pick different directories for the GRE to install to?

*** This bug has been marked as a duplicate of 314592 ***

As the maintainer of the Pidgin GTK+ package, I would like to clarify a few things.

The packages that we distribute are based on the binaries that Tor uploads to ftp.gnome.org.  With the exception of very rare occasions where we apply bugfixes that have already been made upstream, we do not release patched versions.

I think it isn't fair to say that "Gaim/Pidgin is known to ship broken versions of GTK+" - the issue in bug #314529 (which was actually caused by GTK+ bug triggered by an updated GTK-WIMP theme) was unfortunate and as soon as we were alerted of it, we provided an updated package.  I'm not aware of any other situations where we've released "broken" versions.

This is *not* a duplicate of #314529 (as you said that GTK+ bug has long been fixed).  I assure you that the GTK+ version in our 2.10.13 installer is the plain old unpatched binary from ftp.gnome.org.

I get the following stacktrace when I trigger the crash as Ankit describes:

It looks like it is the classic printf("%s", NULL) scenario.  Some gtkrc probably file is missing something that is assumed to be present.

gimp-2.2.exe caused an Access Violation at location 77c478c0 in module MSVCRT.dll Reading from location 00000000.

Registers:
eax=00000000 ebx=00000000 ecx=00000000 edx=00000000 esi=00000000 edi=00000000
eip=77c478c0 esp=0022df5c ebp=0022e2d8 iopl=0         nv up ei pl zr na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000246

Call stack:
77C478C0  MSVCRT.dll:77C478C0  strlen
6730D477  libglib-2.0-0.dll:6730D477  g_win32_locale_filename_from_utf8
67304B14  libglib-2.0-0.dll:67304B14  g_vasprintf
672F4B16  libglib-2.0-0.dll:672F4B16  g_strdup_vprintf
672E59CE  libglib-2.0-0.dll:672E59CE  g_logv
672E5BF6  libglib-2.0-0.dll:672E5BF6  g_log
62760EA7  libgobject-2.0-0.dll:62760EA7  g_type_check_instance_cast
6049FE9E  libgtk-win32-2.0-0.dll:6049FE9E  gtk_button_get_alignment
62743935  libgobject-2.0-0.dll:62743935  g_closure_invoke
62756FB5  libgobject-2.0-0.dll:62756FB5  g_signal_has_handler_pending
62757D5E  libgobject-2.0-0.dll:62757D5E  g_signal_emit_valist
62757FD6  libgobject-2.0-0.dll:62757FD6  g_signal_emit
6274563E  libgobject-2.0-0.dll:6274563E  g_object_interface_list_properties
6274904B  libgobject-2.0-0.dll:6274904B  g_object_thaw_notify
605E3BB8  libgtk-win32-2.0-0.dll:605E3BB8  gtk_rc_property_parse_border
605CA42D  libgtk-win32-2.0-0.dll:605CA42D  gtk_rc_reparse_all_for_settings
605CA6C3  libgtk-win32-2.0-0.dll:605CA6C3  gtk_rc_reparse_all
62743935  libgobject-2.0-0.dll:62743935  g_closure_invoke
62756FB5  libgobject-2.0-0.dll:62756FB5  g_signal_has_handler_pending
62757D5E  libgobject-2.0-0.dll:62757D5E  g_signal_emit_valist
62757FD6  libgobject-2.0-0.dll:62757FD6  g_signal_emit
6274563E  libgobject-2.0-0.dll:6274563E  g_object_interface_list_properties
62748D29  libgobject-2.0-0.dll:62748D29  g_object_set_valist
62748DEF  libgobject-2.0-0.dll:62748DEF  g_object_set
004358D9  gimp-2.2.exe:004358D9
62756FB5  libgobject-2.0-0.dll:62756FB5  g_signal_has_handler_pending
62757D5E  libgobject-2.0-0.dll:62757D5E  g_signal_emit_valist
62757FD6  libgobject-2.0-0.dll:62757FD6  g_signal_emit
6067D830  libgtk-win32-2.0-0.dll:6067D830  gtk_tree_view_scroll_to_cell
60687183  libgtk-win32-2.0-0.dll:60687183  gtk_tree_view_set_model
60578E62  libgtk-win32-2.0-0.dll:60578E62  gtk_marshal_VOID__UINT_STRING
62743935  libgobject-2.0-0.dll:62743935  g_closure_invoke
62756BE6  libgobject-2.0-0.dll:62756BE6  g_signal_has_handler_pending
62757ABC  libgobject-2.0-0.dll:62757ABC  g_signal_emit_valist
62757FD6  libgobject-2.0-0.dll:62757FD6  g_signal_emit
6069C434  libgtk-win32-2.0-0.dll:6069C434  gtk_widget_activate
60576011  libgtk-win32-2.0-0.dll:60576011  gtk_propagate_event
605774A3  libgtk-win32-2.0-0.dll:605774A3  gtk_main_do_event
6B070C5E  libgdk-win32-2.0-0.dll:6B070C5E  gdk_event_get_graphics_expose
672DEA67  libglib-2.0-0.dll:672DEA67  g_main_context_dispatch
672DFF3B  libglib-2.0-0.dll:672DFF3B  g_main_context_acquire
672E011A  libglib-2.0-0.dll:672E011A  g_main_loop_run

Daniel, our bug-tracker is full of bug reports from people who see crashes using the GTK+ packages shipped with Gaim. These crashes go away when they use the GTK+ package that Jernej distributes. So from our point of view it looks like Gaim (and now Pidgin) quite frequently manages to ship broken versions of GTK+. For that reason the GIMP 2.4 installer is going to install and use it's own copy of GTK+. It would probably be best if you did the same. Sharing a GTK+ installation obviously does not work on the Windows platform.

Sorry if this is a stupid question, but to get back to the actual problem reported, what are the "Default" and "Small" themes? Windows themes? GTK+ themes? And how exactly do you change between them?

I'm not really sure what they are. I just know that they're in the default Wimp installation. To change, I went to File > Preferences > Theme and then there are two themes listed: C:\Program Files\GIMP-2.0\share\gimp\2.0\themes\Default and C:\Program Files\GIMP-2.0\share\gimp\2.0\themes\Small

(In reply to comment #12)
> I'm not really sure what they are. I just know that they're in the default Wimp
> installation. To change, I went to File > Preferences > Theme and then there
> are two themes listed: C:\Program Files\GIMP-2.0\share\gimp\2.0\themes\Default
> and C:\Program Files\GIMP-2.0\share\gimp\2.0\themes\Small
> 

Oops. I meant default GIMP installation. Not default Wimp installation. Sorry for the confusion.

After a fair amount of work here comparing the Pidgin installer to the GIMP installer, I've discovered what the difference is that causes the crash (you're going to love this :) ).

Despite the fact that there are some version differences between a few components in the two installers (e.g. Atk, Pango, fontconfig and freetype), none of that seemed to make any difference as I switched dlls to try to track down which one of the difference was causing the crash.

It turns out that what is causing the crash (and I don't know exactly why this is yet) is the .../etc/gtk-2.0/gtkrc file.

Pidgin's is very simple and contains two lines:
gtk-font-name = "sans 8"
gtk-theme-name = "MS-Windows"

GIMP's is a copy of .../share/themes/MS-Windows/gtk-2.0/gtkrc (identical to Pidgin's file in that directory).

When I replace the default gtkrc file with the MS-Windows theme gtkrc file as is done with the GIMP installer, the crash doesn't occur.

What is here?

Ok, I think I've figured out what is happening here (and it is a nice bug that I'm pretty sure I've seen happen many many times before in different scenarios).

This would have been far easier if gdb didn't crash when trying to either run a new or attach to an existing gimp process.

I was curious why I would see stack traces that contain g_type_check_instance_cast(), and now it makes sense.

In the theme file reparse process, gtk_rc_property_parse_border() is eventually called (I didn't follow it exactly to figure out for which specific property, but that isn't important for this issue) with an invalid GParamSpec parameter.  The normal sanity checking calls G_IS_PARAM_SPEC(), which eventually calls g_type_check_instance_cast().  In g_type_check_instance_cast() one of the first two g_warning() calls is hit and type_descriptive_name_I() returns a NULL (because g_quark_to_string() returns NULL if the quark is an unsupported value) this causes the dreaded win32 msvcrt sprintf("%s", NULL) type of crash.

A solution is make sure to that NODE_NAME() returns a non-NULL value so that the g_warning()s don't crash.  I hacked it simply to be (obviously suboptimal because it needs to lock the mutex twice):
#define NODE_NAME(node) (g_quark_to_string (node->qname) ? g_quark_to_string (node->qname) : "(null)")

After doing this, I see some interesting output to stderr when the crash would have occurred:

(gimp-2.2.exe:5644): GLib-GObject-WARNING **: invalid uninstantiatable type `(null)' in cast to `GtkContainer'

Of course, there is still the question of what is passing invalid data into gtk_rc_property_parse_border(), but that is a slightly different issue.

Created attachment 92477 [details] [review]
Patch so that glib's internal snprintf("%s", NULL) behavior is consistent with the linux behavior

The analysis that I did to track down the source of the snprintf("%s", NULL) got me thinking and after digging a bit, I realized that it is actually glib's internal snprintf() that can't handle formatting NULL char* pointers.  I had assumed it was Windows' sprintf(), but fortunately (and unfortunately because I wish I had noticed this sooner) this isn't the case and consequently it is easily fixable.

This fix changes the behavior so that it doesn't choke and instead prints "(null)" like the linux sprintf() would in this situation.

It's glibc that is broken if it accepts NULL for %s.

(In reply to comment #17)
> It's glibc that is broken if it accepts NULL for %s.

Actually, the Windows _snprintf() (and sprintf()) behaves in the same way (I didn't realize this until I just tested it).

(In reply to comment #17)
> It's glibc that is broken if it accepts NULL for %s.

Windows, OS X and NetBSD all behave the same as linux, the only platform I have access to that doesn't accept NULL for %s is solaris 9 (I don't have access to any newer versions, so for all I know, the behavior could be different there).

I don't have a copy of the ISO C spec. handy, but my understanding is that behavior isn't defined for this scenario.  Given that it is a common occurrence (particularly common in logging code, in my experience) that code doesn't guard against such things, it seems like it would be a good thing not to crash when this happens.

I will also put together a patch for gobject/gtype.c so that it doesn't ever try to format a NULL pointer, but I really think it would be valuable (and save a significant amount of time for a lot of people) to add the check to the internal snprintf().

If we have a choice how to do this, why not choose the path less likely to be painful :) ?

Created attachment 92836 [details] [review]
Patch so that NODE_NAME() in gobject/gtype.c always returns a non-null string

Here is the additional patch I promised to prevent the NODE_NAME() from returning a NULL string.

Actually, I don't understand what is the problem in accepting NULL is all about.  Legal programs (that don't pass NULL) will continue to work whether you coredump on NULL, format disk or print (null).  For all other cases, it will save a ton of debugging time.

If you really want to keep 100% compatibility with some standard, don't accept NULL, but just add g_return_if_false (string != NULL) or something like that and _don't crash_.  At least, make error explicit, so that the programmer immediately understands what is wrong, not having to stare at the backtrace of the crash.

*** Bug 470277 has been marked as a duplicate of this bug. ***

As GLib's glib/gnulib/vasnprintf.c nowadays *does* produce "(null)" when passing a NULL pointer for %s (bug #167569, patch applied on 2009-02-27), I guess this problem here should be gone too, then. Resolving as duplicate of bug #167569.

*** This bug has been marked as a duplicate of bug 167569 ***