660131 - crash xpcshell with --enable-gio option on mochitest-plain

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

When run mochitest-plain, http server xpcshell crashes.

#0  0x0000000000000000 in ?? ()
#1  0x00007fffef0bafc3 in g_content_type_guess (filename=0x2248828 "file.js", data=0x0, data_size=0, result_uncertain=0x7fffffff958c)
    at /build/buildd/glib2.0-2.28.6/./gio/gcontenttype.c:908
#2  0x00007fffe9bd0c6f in nsGIOService::GetMimeTypeFromExtension (this=<value optimized out>, aExtension=..., aMimeType=...)
    at /home/zoe/hg/mozilla-central/toolkit/system/gnome/nsGIOService.cpp:322

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

xpcshell does not depend on GTK+ so we need to call g_type_init in nsGIOService::Init() explicitly.

Comment 2

[Karl Tomlinson (:karlt)]

This patch fixes nsGIOService, thanks.

nsGSettingsService also needs g_type_init to be called before it calls
g_settings_new.

In fact every service provided by kGnomeModule needs g_type_init to be called,
although this is done implicitly for two services:
nsAlertsIconListener::InitAlertAsync() calls notify_init() calls g_type_init().
nsGnomeVFSService::Init() calls gnome_vfs_init() calls g_type_init().

Still nsGConfService, nsGSettingsService, and nsGIOService need the explicit
call.  (Only nsGConfService::Init() already calls g_type_init()).

What do you think about instead initializing the loadProc of kGnomeModule to a static function that calls g_type_init?
http://hg.mozilla.org/mozilla-central/annotate/a18b9861a868/toolkit/system/gnome/nsGnomeModule.cpp#l114
http://hg.mozilla.org/mozilla-central/annotate/ce0dfa63db1b/xpcom/components/Module.h#l132

g_type_init could then be removed from nsGConfService, and
nsGIOService::Init() can be removed and
NS_GENERIC_FACTORY_CONSTRUCTOR_INIT replaced with
NS_GENERIC_FACTORY_CONSTRUCTOR.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #2)
> This patch fixes nsGIOService, thanks.
> 
> nsGSettingsService also needs g_type_init to be called before it calls
> g_settings_new.
> 
> In fact every service provided by kGnomeModule needs g_type_init to be
> called,
> although this is done implicitly for two services:
> nsAlertsIconListener::InitAlertAsync() calls notify_init() calls
> g_type_init().
> nsGnomeVFSService::Init() calls gnome_vfs_init() calls g_type_init().
> 
> Still nsGConfService, nsGSettingsService, and nsGIOService need the explicit
> call.  (Only nsGConfService::Init() already calls g_type_init()).
> 
> What do you think about instead initializing the loadProc of kGnomeModule to
> a static function that calls g_type_init?
> http://hg.mozilla.org/mozilla-central/annotate/a18b9861a868/toolkit/system/
> gnome/nsGnomeModule.cpp#l114
> http://hg.mozilla.org/mozilla-central/annotate/ce0dfa63db1b/xpcom/components/
> Module.h#l132
> 
> g_type_init could then be removed from nsGConfService, and
> nsGIOService::Init() can be removed and
> NS_GENERIC_FACTORY_CONSTRUCTOR_INIT replaced with
> NS_GENERIC_FACTORY_CONSTRUCTOR.

Wow! Thank you for your a sharp and an awesome suggestion. I did not know the  LoadFuncPtr.  I will attach a new patch to use it later.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Revised patch

Unfortunately nsGConfService::Init() could not be removed since the method does also initialize mClient but this patch gets better than the previous one.

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 536497 [details] [diff] [review]
Revised patch

Yes, looks good.  Thanks!

Comment 6

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/projects/cedar/rev/d6a897278194

Comment 7

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/d6a897278194

Comment 8

[Stefan]

Does not compile:

.../toolkit/system/gnome/nsGnomeModule.cpp: In function 'nsresult InitGType()':
.../toolkit/system/gnome/nsGnomeModule.cpp:113:15: error: 'g_type_init' was not declared in this scope
make[5]: *** [nsGnomeModule.o] Error 1

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: follow up fix

(In reply to comment #8)
> Does not compile:
> 
> .../toolkit/system/gnome/nsGnomeModule.cpp: In function 'nsresult
> InitGType()':
> .../toolkit/system/gnome/nsGnomeModule.cpp:113:15: error: 'g_type_init' was
> not declared in this scope
> make[5]: *** [nsGnomeModule.o] Error 1

I am sorry for my fault. Are you disabling gconf, right?

Comment 10

[Stefan]

Yes, --disable-gconf is in effect. Unfortunately it does still not compile with the follow-up fix. Including <gconf/gconf-client.h> instead of <glib.h> works.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #10)
> Yes, --disable-gconf is in effect. Unfortunately it does still not compile
> with the follow-up fix. Including <gconf/gconf-client.h> instead of <glib.h>
> works.

Ah! My dumb head! We should include glib/gobject.h actually.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Attachment: correct fix

I am sorry for my bonehead!

The correct header was glib-object.h.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Now I confirmed that building succeeded with --disable-gconf option.

Thank you, Stefan.

Comment 14

[Mounir Lamouri (:mounir)]

Follow-up pushed to cedar.

Comment 15

[Olli Pettay [:smaug]]

http://hg.mozilla.org/mozilla-central/rev/681978590fa6