The patch looks good to me; but I'll let kris commit it, its his code.

(In reply to comment #0)
> It is likely that a few checks are missing in the GtkTooltip's implementation.
> Related segfaults were "randomly" met while strolling the mouse on a
> GtkTreeView coupled with GtkTooltip (as in gtk+-2.12.0/tests/testtooltip.c).
> The actual code is here:
> http://svn.gna.org/viewcvs/lablgtk/trunk/examples/tooltip.ml?rev=1365&sortdir=down&view=markup


Were you able to reproduce these segfaults with the testtooltip C test case?  If not, are you able to produce a testcase in C reproducing the segfaults?  Personally, I haven't seen any segfaults like this when testing tooltips on GtkTreeView.


> Index: gtk/gtktooltip.c                                                         
> ===================================================================
> --- gtk/gtktooltip.c    (revision 18847)
> +++ gtk/gtktooltip.c    (working copy)
> @@ -843,6 +843,9 @@
>    tooltip = g_object_get_data (G_OBJECT (display),
>                                "gdk-display-current-tooltip");
> 
> +  if (!tooltip)
> +    return;
> +
>    if (tooltip->keyboard_mode_enabled)
>      {
>        pointer_widget = tooltip_widget = tooltip->keyboard_widget;
> @@ -867,8 +870,6 @@
> 
>    g_object_get (tooltip_widget, "has-tooltip", &has_tooltip, NULL);
> 
> -  g_assert (tooltip != NULL);
> -
>    return_value = gtk_tooltip_run_requery (&tooltip_widget, tooltip, &x, &y);
>    if (!return_value)
>      return;

I agree it makes sense to move this up, but it should stay a g_assert().  tooltip is really never expected to be NULL here.

> @@ -975,7 +976,8 @@
> 
>    tooltip = g_object_get_data (G_OBJECT (display),
>                                "gdk-display-current-tooltip");
> -  tooltip->timeout_id = 0;
> +  if (tooltip)
> +    tooltip->timeout_id = 0;
> 
>    return FALSE;
>  }

This one calls gtk_tooltip_show_tooltip(), so once we figure out why tooltip is NULL this would be fixed as well.  It might actually make sense to move the assertion from gtk_tooltip_show_tooltip() to here.

> @@ -990,7 +992,7 @@
>    tooltip = g_object_get_data (G_OBJECT (display),
>                                "gdk-display-current-tooltip");
> 
> -  if (tooltip && GTK_TOOLTIP_VISIBLE (tooltip))
> +  if (!tooltip || GTK_TOOLTIP_VISIBLE (tooltip))
>      return;
> 
>    if (tooltip->timeout_id)
> 

Makes sense ;)  Seems like a think-o went it here :)

Created attachment 96127 [details] [review]
updated patch

This would be the updated patch with diff -up.  I think I want to hold off committing this patch for now until we now in which case tooltip appears to be NULL; the problem might be in a different area.

(In reply to comment #2)
> Were you able to reproduce these segfaults with the testtooltip C test case?
Unfortunately not. 

> If not, are you able to produce a testcase in C reproducing the segfaults?
No, everything is fine with a pure C code. 

> Personally, I haven't seen any segfaults like this when testing tooltips on
> GtkTreeView.
Using your updated patch, I have put fprintf/fflush in the callbacks set by gdk_threads_add_timeout_full and also before the calls to gdk_threads_add_timeout_full (suffix "add").
As far as I tried, the good trace is:
  tooltip_browse_mode_expired
  tooltip_popup_timeout add (several times usually)
  tooltip_popup_timeout

Whereas the bad trace is:
  tooltip_browse_mode_expired
  tooltip_popup_timeout
  Gtk-ERROR **: file gtktooltip.c: line 982 (tooltip_popup_timeout): assertion failed: (tooltip != NULL)
  aborting...

See, no "tooltip_popup_timeout add" happened between "tooltip_browse_mode_expired" and "tooltip_popup_timeout"; this should explain why "tooltip_popup_timeout" retrieves a NULL tooltip.

I have no idea why it seems to happen only with OCAML code,
but it might be related to the OCAML way of handling threads.

Hope this helps.

Hi,

  recently I hit this bug while debugging a SEGV in a small example written in Lua (with LuaGnome).  Same issue with Ruby-Gnome2, whereas PythonGtk2 works fine.

  A complete explanation can be found here: 

http://luaforge.net/tracker/index.php?func=detail&aid=24129&group_id=121&atid=576

  Anyway I think the changes proposed above would remedy the problem.  The SEGV happened in the following line of function gtk_tooltip_show_tooltip:

   if (tooltip->keyboard_mode_enabled)

Hope to help,
Wolfgang

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

(In reply to comment #5)
> http://luaforge.net/tracker/index.php?func=detail&aid=24129&group_id=121&atid=576


This server seems down at the moment.. Any chance of putting the explanation and/or test cases in this bugzilla?

The fact that only language bindings seem to be affected might confirm a reference counting issue.  When the tooltip is destroyed from the browse mode expiration timeout, is gtk_tooltip_finalize() called or not?

I'd like to add another "Me too", but this time using GtkTooltip via C# (Gtk.Tooltip) on a custom widget.

I'm getting the same stacktrace:

(gdb) where

+ Trace 208922

And I had suspected the same conclusion (that the tooltip_browse_mode_expired callback was firing, immediately followed by the tooltip_popup_timeout callback)

Kris, since you suspect that the finalize isn't happening... maybe there's uncollected stuff from the language binding keeping the reference count high enough that gtk_tooltip_finalize isn't properly called?

Ok! So based on the hint from Kris about gtk_tooltip_finally, I added an explicit call to the Dispose () method of the gtk# Gtk.Tooltip object at the end of my "query-tooltip" callback, and the crasher seems to be gone!

In the short term, I'm going to suggest to Mike Kestner (gtk# maintainer) that we somehow ensure the bound GtkTooltip is unref'd explicitly at the end of the managed "query-tooltip" callback, to prevent such issues.

I've filed a bug against Gtk# at https://bugzilla.novell.com/show_bug.cgi?id=514476

I have committed the patch from comment 3, but with the assert removed so it basically looks more like the patch from the opening comment.  I have changed my mind on this; even though the case should not happen and you actually want to assert, it can happen through language bindings because of their different ref counting / memory management.  Still GTK+ should guard for this and not crash.