GNOME Bugzilla – Bug 638017
[PATCH] GtkTextView: Crash in gtk_text_view_set_tabs()
Last modified: 2011-01-11 02:13:59 UTC
Created attachment 177050 [details] [review] Hack that fixes the crash The crash happened when I started anjuta with latest gtk+. It seems related to style changes and the attached hack fixes it. It seems default_style might be NULL in some conditions. I am pretty sure that code needs to be reworked anyway.
*** Bug 638113 has been marked as a duplicate of this bug. ***
bug #638118 has the stacktrace which I forgot to report here...
Hmm, the only function that sets priv->layout also calls gtk_text_layout_set_default_style, so I don't see how that could happen. do you have a stacktrace ?
As written in comment 2 there is a stacktrace in bug #638118. It may appear because there is simply no default style because themes, etc. couldn't be build correctly or don't work.
This seems to affect everybody you tries to run anjuta with jhbuild gtk+ so I would very much appreciate if the patch was applied or if someone could explain why this shouldn't be a problem in gtk+.
Created attachment 177317 [details] [review] another fix Indeed the only function (gtk_text_view_ensure_layout) that sets priv->layout calls gtk_text_layout_set_default_style but after doing other things. As we can see in the stack trace (bug 638113), these "other things" emit some signals and one of this signal call the gtk_text_view_set_tabs function before the default style is initialized leading to a crash. I have tried to write another fix, setting the widget layout only when the default style is set. The idea is quite similar to Johannes's patch but done in a different way. Another possibility could be to fix gtksourceview to avoid calling the gtk_text_view_set_tabs in this situation but I don't know how to detect it and I'm afraid it's a bit dangerous anyway.
Review of attachment 177317 [details] [review]: This patch screams recursion to me, since you are still connecting the signal handlers early, but no longer set priv->layout. So if any of the signal handlers gets triggered during the initialization of the layout, you may reenter gtk_text_view_ensure_layout, with obviously bad consequences. The patch would make more sense to me if you moved the signal connections down below the setting of priv->layout as well.
Created attachment 177425 [details] [review] fix creating style as soon as possible Indeed it doesn't work if the signal handlers uses the layout. A few functions called here like gtk_text_layout_set_cursor_visible are emitting signals too so their handlers shouldn't call gtk_text_view_ensure_layout neither. So moving the signal connections is not enough. I have rather moved upward the initialization of the style object just after creating and setting the layout.
Review of attachment 177425 [details] [review]: Yeah, this looks good to me
Attachment 177425 [details] doesn't work for me: (gdb) run Starting program: /home/jhs/gnome-unstable/bin/anjuta [Thread debugging using libthread_db enabled] [New Thread 0xb7be0b70 (LWP 16027)] [New Thread 0xb71ffb70 (LWP 16028)] [New Thread 0xb2c34b70 (LWP 16029)] [New Thread 0xb22ffb70 (LWP 16030)] [Thread 0xb22ffb70 (LWP 16030) exited] (anjuta:16024): Gtk-CRITICAL **: gtk_style_provider_get_style_property: assertion `g_type_is_a (gtk_widget_path_get_object_type (path), pspec->owner_type)' failed (anjuta:16024): Gtk-CRITICAL **: gtk_style_provider_get_style_property: assertion `g_type_is_a (gtk_widget_path_get_object_type (path), pspec->owner_type)' failed (anjuta:16024): Gtk-CRITICAL **: gtk_style_provider_get_style_property: assertion `g_type_is_a (gtk_widget_path_get_object_type (path), pspec->owner_type)' failed (anjuta:16024): Gtk-CRITICAL **: gtk_style_provider_get_style_property: assertion `g_type_is_a (gtk_widget_path_get_object_type (path), pspec->owner_type)' failed (anjuta:16024): Gtk-CRITICAL **: gtk_style_provider_get_style_property: assertion `g_type_is_a (gtk_widget_path_get_object_type (path), pspec->owner_type)' failed (anjuta:16024): Gtk-CRITICAL **: gtk_style_provider_get_style_property: assertion `g_type_is_a (gtk_widget_path_get_object_type (path), pspec->owner_type)' failed [New Thread 0xb22ffb70 (LWP 16031)] Detaching after fork from child process 16032. [New Thread 0xb1afeb70 (LWP 16033)] Detaching after fork from child process 16034. Detaching after fork from child process 16035. Detaching after fork from child process 16036. Detaching after fork from child process 16037. Detaching after fork from child process 16038. Program received signal SIGSEGV, Segmentation fault. 0x00420573 in gtk_text_view_set_tabs (text_view=0x8b66048, tabs=0x825aaa0) at gtktextview.c:2824 2824 if (priv->layout->default_style->tabs) Missing separate debuginfos, use: debuginfo-install freetype-2.4.2-4.fc14.i686 glibc-2.12.90-21.i686 libX11-1.3.4-3.fc14.i686 libXau-1.0.6-1.fc14.i686 libXcomposite-0.4.2-1.fc14.i686 libXcursor-1.1.10-5.fc14.i686 libXdamage-1.1.3-1.fc14.i686 libXext-1.1.2-2.fc14.i686 libXfixes-4.0.5-1.fc14.i686 libXi-1.3.2-1.fc14.i686 libXinerama-1.1-2.fc13.i686 libXrandr-1.3.0-5.fc13.i686 libXrender-0.9.6-1.fc14.i686 libgcc-4.5.1-4.fc14.i686 libogg-1.2.0-1.fc14.i686 libpng-1.2.44-1.fc14.i686 libselinux-2.0.96-6.fc14.1.i686 libudev-161-8.fc14.i686 libvorbis-1.3.1-2.fc14.i686 libxcb-1.7-1.fc14.i686 zlib-1.2.5-2.fc14.i686 (gdb) bt
+ Trace 225389
Indeed the patch is wrong but I don't see an easy fix. I have tried to set together priv->layout and priv->layout->default_style so only the already existing check if priv->layout is NULL is enough. My first patch set both of them at the end of the initialization but it will not work if a signal handler call gtk_text_view_ensure_layout during the initialization sequence. The second patch set both of them at the beginning but after calling gtk_text_view_set_attributes_from_style which is needed for initializing the style and it's already too late. Perhaps the best solution is to do what is done in Johannes patch, I mean checking if default_style is null, but in all functions using it.
I probably found why this happens: In it's GtkTextView subclass, anjuta calls g_object_set ("tab-width", 4) in _init(). I don't think that should crash but it doesn't if "tab-width" is set within g_object_new ().
How do you trigger the crash? We do that in gedit too and here it doesn't crash: http://git.gnome.org/browse/gedit/tree/gedit/gedit-view.c#n380
Doing anything but simple assignments in init is basically always wrong. But I agree that it should not crash...
Created attachment 177617 [details] [review] Another fix, checking layout->default_style before using it Here is another patch checking if layout->default_style is NULL before using it, like in Johannes's patch but done in other GtkTextView functions. I have looked in other files but I think there is no other place where this check is needed except perhaps in the function get_style in gtk/gtktextlayout.c. I have get the crash with Gtk+ commit 48b47971b5c6657a8141b9128c5dc05bfa80ced7. I don't get it anymore with the latest version of Gtk+ and Johannes has committed a change in Anjuta to avoid it so I don't know if it's really needed to apply it.