Bug 380339 - multiple slabs ...
Summary: multiple slabs ...
Status: RESOLVED FIXED
Alias: None
Product: openSUSE 11.0
Classification: openSUSE
Component: GNOME (show other bugs)
Version: Factory
Hardware: Other Other
: P5 - None : Major (vote)
Target Milestone: ---
Assignee: Federico Mena Quintero
QA Contact: E-mail List
URL:
Whiteboard: gnome-crash,gnomeup-gnome-main-menu
Keywords:
Depends on:
Blocks: main-menu-behavior randr-tracker
  Show dependency treegraph
 
Reported: 2008-04-16 09:52 UTC by Michael Meeks
Modified: 2008-04-23 21:57 UTC (History)
2 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
gnome-main-menu-bnc380339-multiple-applet-crash.diff (998 bytes, patch)
2008-04-23 21:54 UTC, Federico Mena Quintero
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2008-04-16 09:52:35 UTC
add two of the new slab menus to the panel; click one or the other:

** ERROR:(libslab-utils.c:582):create_thumbnail_factory: assertion failed: (thumbnail_factory == NULL)

bang ;-)
Comment 1 Magnus Boman 2008-04-22 05:43:34 UTC
Why do we have to bail out if it's NULL?

Did a simple test, as per below, and it works just fine if thumbnail_factory is NULL.

static void
create_thumbnail_factory (void)
{
 libslab_checkpoint ("create_thumbnail_factory(): start");

// g_assert (thumbnail_factory == NULL);
 thumbnail_factory = NULL;
 thumbnail_factory = gnome_thumbnail_factory_new (GNOME_THUMBNAIL_SIZE_NORMAL);

 libslab_checkpoint ("create_thumbnail_factory(): end");
}
Comment 2 Michael Meeks 2008-04-22 09:23:16 UTC
Magnus - that would leak horribly (presumably) ;-)

A quick fix might be to do g_return_if_fail (thumbnail_factory == NULL); instead of a g_assert (which seems a little harsh).

It seems a little over-enthusiastic to not only assert the pointer is NULL, but then to NULL it as well in the next line, before assigning it :-)
Comment 3 Magnus Boman 2008-04-22 09:33:31 UTC
Heh... Sorry that I wasn't clear in my comment above.

I commented out g_assert (thumbnail_factory == NULL);
and added thumbnail_factory = NULL; 
Just to see if it would work if it was NULL.

My suggestion is that the code below is sufficient;

static void
create_thumbnail_factory (void)
{
 libslab_checkpoint ("create_thumbnail_factory(): start");

 thumbnail_factory = gnome_thumbnail_factory_new (GNOME_THUMBNAIL_SIZE_NORMAL);

 libslab_checkpoint ("create_thumbnail_factory(): end");
}
Comment 4 Federico Mena Quintero 2008-04-22 16:23:56 UTC
The problem is that src/main-menu.c:main_menu_applet_init(), which is the applet's factory function, calls libslab_thumbnail_factory_preinit().  This then gets called a second time for the second slab.

The right solution is to do something like

libslab_thumbnail_factory_preinit ()
{
    if (thumbnail_factory_idle_id != 0)
	thumbnail_factory_idle_id = g_idle_add (init_thumbnail_factory_idle_cb, NULL);
}

instead of unconditionally setting up the idle handler.

(The assertion you ran into means "I don't want to create the thumbnail factory twice, to avoid leaking --- it's really doing what it intends to do :)
Comment 5 Federico Mena Quintero 2008-04-23 21:49:55 UTC
Heh... Captain_magnus was right after all :)  With my fix above, it asserts if you add a second instance of the applet.  Attaching the patch I'll commit.
Comment 6 Federico Mena Quintero 2008-04-23 21:54:44 UTC
Created attachment 210006 [details]
gnome-main-menu-bnc380339-multiple-applet-crash.diff
Comment 7 Federico Mena Quintero 2008-04-23 21:57:50 UTC
Submitted to autobuild.  This should make it into Beta2.

* Wed Apr 23 2008 - federico@novell.com
- Added gnome-main-menu-bnc380339-multiple-applet-crash.diff to fix
  https://bugzilla.novell.com/show_bug.cgi?id=380339 - Crash when
  there are multiple instances of main-menu in the panel.