Last Comment Bug 737084 - Do pthread_atfork in jemalloc on mac and android
: Do pthread_atfork in jemalloc on mac and android
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: mozglue (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on: 738559 738561 738709
Blocks: jemalloc3
  Show dependency treegraph
 
Reported: 2012-03-19 10:23 PDT by Mike Hommey [:glandium]
Modified: 2012-04-02 11:28 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Do pthread_atfork in jemalloc on mac and android (7.00 KB, patch)
2012-03-19 10:40 PDT, Mike Hommey [:glandium]
khuey: review+
blassey.bugs: review+
justin.lebar+bug: feedback+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2012-03-19 10:23:57 PDT
Currently, the pthread_atfork call from jemalloc is only done on Linux. There used to be a workaround in the ipc code, around a fork, but it went away recently. Anyways, to avoid any future surprise around this, we'd be better off doing pthread_atfork on Android, which "only" requires a few wrapped functions.
And we should also do it on mac, since that's supported.
Comment 1 Mike Hommey [:glandium] 2012-03-19 10:40:50 PDT
Created attachment 607211 [details] [diff] [review]
Do pthread_atfork in jemalloc on mac and android
Comment 2 Justin Lebar (not reading bugmail) 2012-03-20 08:48:05 PDT
I don't pretend to know exactly what's going on here, but 

         WRAP_LDFLAGS="${WRAP_LDFLAGS} -Wl,--wrap=getaddrinfo,--wrap=freeaddrinfo,--wrap=gai_strerror"
+        WRAP_LDFLAGS="${WRAP_LDFLAGS} -Wl,--wrap=fork,--wrap=pthread_atfork"

the second line doesn't overwrite the first one here?
Comment 3 Mike Hommey [:glandium] 2012-03-20 09:13:33 PDT
(In reply to Justin Lebar [:jlebar] from comment #2)
> I don't pretend to know exactly what's going on here, but 
> 
>          WRAP_LDFLAGS="${WRAP_LDFLAGS}
> -Wl,--wrap=getaddrinfo,--wrap=freeaddrinfo,--wrap=gai_strerror"
> +        WRAP_LDFLAGS="${WRAP_LDFLAGS} -Wl,--wrap=fork,--wrap=pthread_atfork"
> 
> the second line doesn't overwrite the first one here?

It's appends each time.
Comment 4 Justin Lebar (not reading bugmail) 2012-03-20 09:20:33 PDT
> It's appends each time.

Wow, it's like they *tried* to make a ridiculous language.
Comment 5 Brad Lassey [:blassey] (use needinfo?) 2012-03-20 18:45:32 PDT
(In reply to Mike Hommey [:glandium] from comment #0)
> Currently, the pthread_atfork call from jemalloc is only done on Linux.
> There used to be a workaround in the ipc code, around a fork, but it went
> away recently. 
What went away?
Comment 6 Mike Hommey [:glandium] 2012-03-20 23:29:13 PDT
(In reply to Brad Lassey [:blassey] from comment #5)
> (In reply to Mike Hommey [:glandium] from comment #0)
> > Currently, the pthread_atfork call from jemalloc is only done on Linux.
> > There used to be a workaround in the ipc code, around a fork, but it went
> > away recently. 
> What went away?

See bug 622992. While this avoids the problem that using the atfork callbacks solves, it doesn't ensure that future changes won't break assumptions again. Or that we won't be doing a fork() for some place else.
Comment 8 Marco Bonardo [::mak] (Away 6-20 Aug) 2012-03-22 18:09:54 PDT
https://hg.mozilla.org/mozilla-central/rev/c6634316f474
Comment 9 Kan-Ru Chen [:kanru] (UTC+8) 2012-03-22 19:56:11 PDT
This breaks b2g build.
Comment 10 Mike Hommey [:glandium] 2012-03-23 00:06:09 PDT
(In reply to Kan-Ru Chen [:kanru] from comment #9)
> This breaks b2g build.

Please file a bug. Do i guess right that it fails to link libmozglue because it can't find pthread_atfork?

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