73053 - Lea memory allocator breaks in most Unices

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

The Lea memory allocator (--enable-lea) doesn't deal with startup issues
properly.  malloc() calls pthread_mutex_lock(), which calls
pthread_mutex_init(), which calls malloc()...  Boom, pthread calls exit().

The solution is to not call pthread_mutex_lock until the lock has been
initialized.  I used a very general procedure patterned after dmalloc
(www.dmalloc.com), but not exactly the same (more general, minimal extra
overhead at runtime).

--enable-lea now works with FreeBSD 4.1-STABLE.  It should work with any
pthreads system; perhaps requiring tuning of the thread-lock_count and
THREAD_INIT_LOCK values.  (I was pretty conservative with the current values.)

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Patch to make lea allocator work with all pthread evironments

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Reassigning to kandrot, cc waterson
Adding crash (since it crashes with --enable-lea on most Unixes)
Nominating for mozilla0.8.1 (it's a pretty safe patch, and only affects
--enable-lea).

Comment 3

[Brendan Eich [:brendan]]

How about diff -u?  My eyes hurt.

/be

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Patch with cvs diff -u

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

BTW, a note that should be made somewhere is that to use LEA currently, you need
to "setenv LD_PRELOAD whatever/libxpcom.so" (or sh equivalent) before running
it.  Running it in GDB is even more fun (but possible).

Alternate memory allocators (Lea or PHK's) should be statically linked into the
main executable to avoid massive hassles; they should not be part of xpcom (it's
too late).

Comment 6

[Håkan Waara]

Maybe someone could r=/sr= the patch in here?

Comment 7

[Edward Kandrot]

Thanks for the patch.  There are two other bugs that currently interfere with me
applying this patch:  lea currently crashes on Linux (ie so therefore Lea
currently does not work at all now), and there is a newer version of Lea.  I
will look into how to add this patch into the new Lea and see if it fixes the
general crashes that other people are now seeing.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

"Crashes on Linux" - are you sure it still crashes with this patch if you do the
LD_PRELOAD magic I mention?  Without both the patch and the LD_PRELOAD, it
_will_ crash on any system.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Any movement on getting the latest Lea allocator in?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

nominating for 0.9.4 to try to get something to move here (I would nominate for
0.9.5 but that's not a valid keyword)

Comment 11

[Edward Kandrot]

I now have a build with 2.7.0 in and it seems to work.  I want to verify it, and
I guess I need r=, sr= to get it checked in.  I will append 2.7.0, with out the
changes for pthread, since so it will be a clean Lea 2.7.0.  Once that is
approved, then we should be able to get the changes approved, if they are still
valid for 2.7.0.

Comment 12

[Edward Kandrot]

Attachment: Lea's 2.7.0 malloc.c. Needs approval to checkin, I guess. r=? sr=?

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Reassigning to waterson (Chris, feel free to reassign again.  I'd take this if I
can get our legal dept to approve my CVS access form.) Sounds like this is
pretty much ready for checkin anyways.  r=? sr=?

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Since it's been 3 months, I'm taking this one.  I'll give the 2.7.0 patch a spin
under Linux & BSD and then ask for reviews.

Adding mlk to put it on memory-use radar, though it's not really a "leak"

Comment 15

[Andreas Franke (gone)]

Lea 2.7 is bug 72497. Since this one is targeted at 0.9.7, and that one is
targeted at 0.9.8, I think someone needs to update both :-)

Comment 16

[Brant Gurganus]

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.

Comment 17

[Andrew Schultz]

*** Bug 191979 has been marked as a duplicate of this bug. ***

Comment 18

[Wayne Mery (:wsmwk)]

shaver, OK to close?

Comment 19

[Mike Shaver (:shaver emeritus)]

Fine with me.

Comment 20

[Wayne Mery (:wsmwk)]

(In reply to comment #15)
> Lea 2.7 is bug 72497. Since this one is targeted at 0.9.7, and that one is
> targeted at 0.9.8, I think someone needs to update both :-)

=> wontfix, given that bug 72497 is wontfix