Last Comment Bug 763358 - (webapprt-stub:8885): GLib-CRITICAL **: g_slice_set_config: assertion `sys_page_size == 0' failed
: (webapprt-stub:8885): GLib-CRITICAL **: g_slice_set_config: assertion `sys_pa...
Status: VERIFIED FIXED
[qa!]
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: Trunk
: x86_64 Linux
: -- normal
: Firefox 16
Assigned To: Marco Castelluccio [:marco]
: Jason Smith [:jsmith]
:
Mentors:
Depends on: 768768
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-10 17:06 PDT by skierpage
Modified: 2016-02-04 15:00 PST (History)
6 users (show)
jsmith: in‑moztrap+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Move gtk_init() to avoid an assert (571 bytes, patch)
2012-06-11 05:05 PDT, John Drinkwater (:beta)
no flags Details | Diff | Splinter Review
Same patch with arguments (1.58 KB, patch)
2012-06-11 09:47 PDT, Marco Castelluccio [:marco]
karlt: review+
Details | Diff | Splinter Review
Set gslice only when needed (2.36 KB, patch)
2012-06-11 11:21 PDT, Marco Castelluccio [:marco]
mh+mozilla: review-
Details | Diff | Splinter Review
Patch (1.58 KB, patch)
2012-06-12 06:06 PDT, Marco Castelluccio [:marco]
mcastelluccio: review+
Details | Diff | Splinter Review
Patch (1.58 KB, patch)
2012-06-13 08:31 PDT, Marco Castelluccio [:marco]
mcastelluccio: review+
Details | Diff | Splinter Review

Description User image skierpage 2012-06-10 17:06:53 PDT
I've installed Private Joe and Lord of Ultima, and when I start either from a terminal I see this message.  Lord of Ultima goes on to run OK despite the "CRITICAL".

Using strace, this warning prints after lstat("/home/skierpage/programs/firefox/webapprt". Both my firefox (nightly profiling) and webapprt-stub are 64-bit.

FWIW bug 672671 is the same warning from elsewhere in the codebase, with a fix.
Comment 1 User image John Drinkwater (:beta) 2012-06-11 05:01:51 PDT
…
lstat("/home/john/bin/firefox-nightly/firefox", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/lib/x86_64-linux-gnu/charset.alias", O_RDONLY) = -1 ENOENT (No such file or directory)
write(2, "\n(webapprt-stub:22023): GLib-CRI"..., 100
(webapprt-stub:22023): GLib-CRITICAL **: g_slice_set_config: assertion `sys_page_size == 0' failed
) = 100

For me its triggering once XRE_main is called, the last lstat is from XRE_GetFileFromPath(firefoxDir).
Comment 2 User image John Drinkwater (:beta) 2012-06-11 05:05:31 PDT
Created attachment 631860 [details] [diff] [review]
Move gtk_init() to avoid an assert

If an error dialog isn’t needed in normal use case, gtk doesn’t need to be inited. So I moved that into the ErrorDialog function and it no longer triggers the assert ‐ XRE_Main expects nothing gtk/glib inited beforehand.
Comment 3 User image Marco Castelluccio [:marco] 2012-06-11 09:47:39 PDT
Created attachment 631926 [details] [diff] [review]
Same patch with arguments

Thanks John.
I don't know if we can do something like this, because we should call gtk_init (or gtk_parse_args) in the main function (because we want gtk to remove its arguments from argc and argv).
Comment 4 User image :Felipe Gomes (needinfo me!) 2012-06-11 10:13:28 PDT
CC'ing Glandium to take a look here, as the comments from the similar bug 672671 say that this error makes jemalloc not work, and he might now what is the proper way to fix it.
Comment 5 User image Marco Castelluccio [:marco] 2012-06-11 10:19:36 PDT
I could add a function to remove the gtk related arguments if this assertion is important (this would be a better solution than the other patch, but would complicate the webapprt code a bit).
Comment 6 User image Mike Hommey [:glandium] 2012-06-11 10:26:03 PDT
(In reply to Marco Castelluccio from comment #3)
> Created attachment 631926 [details] [diff] [review]
> Same patch with arguments
> 
> Thanks John.
> I don't know if we can do something like this, because we should call
> gtk_init (or gtk_parse_args) in the main function (because we want gtk to
> remove its arguments from argc and argv).

XRE_main calls gtk_init already, and I think that's the main reason why you can't call gtk_init before.

(In reply to Felipe Gomes (:felipe) from comment #4)
> CC'ing Glandium to take a look here, as the comments from the similar bug
> 672671 say that this error makes jemalloc not work, and he might now what is
> the proper way to fix it.

I don't think this has any effect on jemalloc. The assertion probably "simply" comes from the double gtk initialization.
Comment 7 User image Mike Hommey [:glandium] 2012-06-11 10:45:49 PDT
(In reply to Mike Hommey [:glandium] from comment #6)
> I don't think this has any effect on jemalloc. The assertion probably
> "simply" comes from the double gtk initialization.

After checking, this comes from g_slice_set_config (in XRE_main) being called after gtk_init (in main).

The error doesn't have an effect on jemalloc, however, it has an effect on gslice memory allocation using its own slab allocator, which may increase memory fragmentation, and thus firefox memory usage.
Comment 8 User image Marco Castelluccio [:marco] 2012-06-11 11:08:36 PDT
(In reply to Mike Hommey [:glandium] from comment #7)
> (In reply to Mike Hommey [:glandium] from comment #6)
> > I don't think this has any effect on jemalloc. The assertion probably
> > "simply" comes from the double gtk initialization.
> 
> After checking, this comes from g_slice_set_config (in XRE_main) being
> called after gtk_init (in main).
> 
> The error doesn't have an effect on jemalloc, however, it has an effect on
> gslice memory allocation using its own slab allocator, which may increase
> memory fragmentation, and thus firefox memory usage.

Indeed the simple double initialization isn't a problem for gtk (they don't do anything on the second initialization).
Comment 9 User image Marco Castelluccio [:marco] 2012-06-11 11:21:16 PDT
Created attachment 631957 [details] [diff] [review]
Set gslice only when needed
Comment 10 User image Mike Hommey [:glandium] 2012-06-11 11:28:34 PDT
Comment on attachment 631957 [details] [diff] [review]
Set gslice only when needed

I preferred the patch that moves gtk initialization into the error function.
Comment 11 User image Marco Castelluccio [:marco] 2012-06-11 11:58:38 PDT
(In reply to Mike Hommey [:glandium] from comment #10)
> I preferred the patch that moves gtk initialization into the error function.

With that method we won't support GTK commandline options. Don't we care about them?
Comment 12 User image Mike Hommey [:glandium] 2012-06-11 12:17:34 PDT
(In reply to Marco Castelluccio from comment #11)
> (In reply to Mike Hommey [:glandium] from comment #10)
> > I preferred the patch that moves gtk initialization into the error function.
> 
> With that method we won't support GTK commandline options. Don't we care
> about them?

Why wouldn't we, since XRE_main does handle them?
Comment 13 User image Marco Castelluccio [:marco] 2012-06-11 13:59:46 PDT
Comment on attachment 631926 [details] [diff] [review]
Same patch with arguments

Oh, you're right! I couldn't find gtk_parse_args with MXR because I was searching for an identifier (and MXR, I don't know why, doesn't handle it as an identifier).
Comment 14 User image Karl Tomlinson (:karlt) 2012-06-11 17:12:02 PDT
Comment on attachment 631926 [details] [diff] [review]
Same patch with arguments

>+  gtk_init(pargc,pargv);

File style is to have spaces between parameters.

This approach also saves having 2 connections to the X11 display.
Fortunately gdk_set_program_class doesn't need a g_type_init.
Comment 15 User image Marco Castelluccio [:marco] 2012-06-12 06:06:07 PDT
Created attachment 632221 [details] [diff] [review]
Patch

Carrying r+
Comment 16 User image Marco Castelluccio [:marco] 2012-06-13 08:31:34 PDT
Created attachment 632706 [details] [diff] [review]
Patch

Updated patch.
Comment 17 User image :Felipe Gomes (needinfo me!) 2012-06-13 10:28:14 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac01424220d8
Comment 18 User image Ed Morley [:emorley] 2012-06-14 02:47:44 PDT
https://hg.mozilla.org/mozilla-central/rev/ac01424220d8
Comment 19 User image Marco Castelluccio [:marco] 2012-06-22 08:26:37 PDT
Jason, I've created a new test case to launch an application through the shell and check that no assertion is fired: https://moztrap.mozilla.org/manage/caseversion/1074/
Comment 20 User image Jason Smith [:jsmith] 2012-06-22 08:52:18 PDT
(In reply to Marco Castelluccio from comment #19)
> Jason, I've created a new test case to launch an application through the
> shell and check that no assertion is fired:
> https://moztrap.mozilla.org/manage/caseversion/1074/

Looks good.

Note You need to log in before you can comment on or make changes to this bug.