! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Memory warning from the xfce4-menu-plugin
Status:
RESOLVED: FIXED
Product:
Xfce4-panel

Comments

Description Daichi Kawahata editbugs 2007-04-14 20:44:49 CEST
User-Agent:       Mozilla/5.0 (X11; U; IRIX64 IP35; ja; rv:1.8.0.8) Gecko/R-A-C Firefox/1.5.0.8
Build Identifier: 

When I tried to add Xfce menu to a panel, it left a log below to my
xsession.log;

  ***MEMORY-WARNING***: xfce4-menu-plugin[179916]: GSlice: g_thread_init()
   must be called before all other GLib functions; memory corruption due
   to late invocation of g_thread_init() has been detected; this program
   is likely to crash, leak or unexpectedly abort soon...

Reproducible: Always

Steps to Reproduce:
1. Select "Add New Item" from the panel menu.
2. Drag "Xfce Menu" to the panel.

Actual Results:  
It warns like above.

Expected Results:  
It should not warn.

It's the latest trunk (rev. 25552) with the latest GTK+ (rev. 17600),
Glib (rev. 5447), GCC 3.4.6.

I've read "Bug 2427 - large memleaks on F5" possibly related, but
its version was "4.3.x" while "4.5.x" here, thus I opened a new
account.
Comment 1 Brian J. Tarricone (not reading bugmail) 2007-05-10 01:32:15 CEST
Problem here is that the plugin's main() function is generated by a macro in the panel source.  Not sure about the best way to fix this.
Comment 2 Nick Schermer editbugs 2007-05-10 08:23:01 CEST
I added a g_thread_init line in the external macro while Xfce was in the RC stage, but there were some comments (Brian and Benny IIRC) it was not needed, so it was removed shortly after the commit.

I still think it can't hurt there and since a lot of plugins use the gslice functions it will stop the memory warnings.
Comment 3 Nick Schermer editbugs 2007-05-10 08:27:11 CEST
Created attachment 1129 
Commit 24260
Comment 4 Brian J. Tarricone (not reading bugmail) 2007-05-10 15:52:51 CEST
Right, but not everyone (using earlier gtk versions) links to libgthread.  Forcing that requirement is kinda lame.
Comment 5 Brian J. Tarricone (not reading bugmail) 2007-05-15 20:50:09 CEST
Nick, Jasper, can we do something to special-case plugins that need to make use of threading?  I see a few

1.  Make the app author responsible for picking the right macro to use, either the current XFCE_PANEL_PLUGIN_REGISTER_EXTERNAL(), or a new _REGISTER_EXTERNAL_THREADED() which does the same stuff, except calls g_thread_init().  Problem: what if they also want gdk_threads_init()?  But what if they *don't* want gdk_threads_init()?

2.  Somehow detect if the plugin is linked against libgthread, and call g_thread_init() if so.  I'm not sure how to do this portably.  This also doesn't solve the case where it's not linked against libgthread, but the plugin is using pthreads directly (the slice allocator will still need g_thread_init() if a slice-using function is called from multiple threads).

3.  Add a new macro, XFCE_PANEL_PLUGIN_REGISTER_EXTERNAL_FULL() which takes a second parameter, which is an "init function" that gets called before gtk_init() (this init function maybe could get argc and argv passed to i).  This way, plugin authors could do fun things with argc/argv before gtk gets ahold of it, or maybe just read options and whatnot (though option parsing is usually more useful after gtk_init() has stripped out the options it cares about).  At any rate, aside from option parsing, a plugin author could call g_thread_init() and/or gdk_threads_init() from the init function if they so desire.  And plugin authors who don't care and don't use threads can just use normal _REGISTER_EXTERNAL().

I kinda like #3 the best.
Comment 6 Jasper Huijsmans editbugs 2007-05-16 18:26:02 CEST
(In reply to comment #5)
...
> 3.  Add a new macro, XFCE_PANEL_PLUGIN_REGISTER_EXTERNAL_FULL() which takes a
> second parameter, which is an "init function" that gets called before
> gtk_init() (this init function maybe could get argc and argv passed to i). 
> This way, plugin authors could do fun things with argc/argv before gtk gets
> ahold of it, or maybe just read options and whatnot (though option parsing is
> usually more useful after gtk_init() has stripped out the options it cares
> about).  At any rate, aside from option parsing, a plugin author could call
> g_thread_init() and/or gdk_threads_init() from the init function if they so
> desire.  And plugin authors who don't care and don't use threads can just use
> normal _REGISTER_EXTERNAL().
> 
> I kinda like #3 the best.
> 

Me too. That does sound very useful and, as you say, possibly for more than just gtk_threads_init().

Comment 7 Jasper Huijsmans editbugs 2007-05-16 19:09:12 CEST
Created attachment 1144 
add XFCE_PANEL_PLUGIN_REGISTER_EXTERNAL_WITH_INIT

Brian, would something like this work for you?
Comment 8 Brian J. Tarricone (not reading bugmail) 2007-05-16 19:26:17 CEST
Yeah, that should work just fine.

I'd probably over-engineer it and add a return value to init_func() so plugin authors could signal a fatal error in some init activity they do.  That might be overkill, though, since presumably they could just as easily do it in their construct function, and at that point they'd have gtk running so they could pop up an error dialog.  A fatal error in init_func() should probably just be handled (by the plugin itself) with a printf() followed by an exit().
Comment 9 Jasper Huijsmans editbugs 2007-05-17 07:53:57 CEST
Created attachment 1145 
overengineered version ;-)

Ok, good point. How about this one? 

One question, should this be added for 4.4.x as well as trunk?
Comment 10 Brian J. Tarricone (not reading bugmail) 2007-05-17 08:16:56 CEST
As I said, I don't think the check thing is necessary; there's no reason why authors couldn't just do that in 'construct' and either xfce_message_dialog() or g_printerr(), followed by exit() on failure.  Unless you want to be able to catch things like that in the panel, I guess.

Yes, this should go in 4.4 as well, otherwise glib 2.10+ users will potentially have problems with any plugin that uses threads (like the menu plugin).
Comment 11 Jasper Huijsmans editbugs 2007-05-17 09:01:02 CEST
I like the check thing and it doesn't harm anyway. I've committed the changes with revision 25719. Feel free to fix things up to better suit your needs, though.
Comment 12 Daichi Kawahata editbugs 2007-05-20 15:42:44 CEST
Created attachment 1149 
Suggested patch

Hi Jasper,

I think I may find a typo, though still might be there;

  desktop-menu-plugin.c:1092: error: called object is not a function
  desktop-menu-plugin.c:1092: warning: unused variable `preinit
Comment 13 Jasper Huijsmans editbugs 2007-05-20 17:50:20 CEST
(In reply to comment #12)
> Hi Jasper,
> 
> I think I may find a typo, though still might be there;

I think you were right ;-) Should be fixed with revision 25735. Thanks.
Comment 14 Daichi Kawahata editbugs 2007-05-27 12:33:46 CEST
(In reply to comment #13)
> (In reply to comment #12)
> > Hi Jasper,
> > 
> > I think I may find a typo, though still might be there;
> 
> I think you were right ;-) Should be fixed with revision 25735. Thanks.

Thanks,

By the way, both of xfce4-panel & xfdesktop are now rev. 25763,
nonetheless, warning still occurs...

Also I've changed the Component & Hardware & OS as it seems
not specific problem on IRIX.

Comment 15 Brian J. Tarricone (not reading bugmail) 2007-07-07 20:57:11 CEST
Ok, finally got around to this...

Bug #3141

Reported by:
Daichi Kawahata
Reported on: 2007-04-14
Last modified on: 2010-11-20

People

Assignee:
Nick Schermer
CC List:
2 users

Version

Version:
4.7 (master)

Attachments

Commit 24260 (186 bytes, text/plain)
2007-05-10 08:27 CEST , Nick Schermer
no flags
add XFCE_PANEL_PLUGIN_REGISTER_EXTERNAL_WITH_INIT (2.33 KB, patch)
2007-05-16 19:09 CEST , Jasper Huijsmans
no flags
overengineered version ;-) (8.31 KB, patch)
2007-05-17 07:53 CEST , Jasper Huijsmans
no flags
Suggested patch (723 bytes, patch)
2007-05-20 15:42 CEST , Daichi Kawahata
no flags

Additional information