727202 - GeckoAppShell.allocateDirectBuffer and freeDirectBuffer don't use jemalloc to allocate/free buffers

Status: RESOLVED FIXED

(Core :: mozglue, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

I noticed this issue when working on bug 726930 because my code malloc'd the buffer directly then called GeckoAppShell.freeDirectBuffer() to free it and that crashed because of the mismatched allocator.

Comment 1

[Mike Hommey [:glandium]]

Sadly, generalization can't be done in mozglue, because some things that we free() are allocated by the android libc. So in the end, this will need to be a localized fix.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Wrap malloc and free to use jemalloc in GeckoAppShell.{allocate,free}DirectBuffer

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 597412 [details] [diff] [review]
Wrap malloc and free to use jemalloc in GeckoAppShell.{allocate,free}DirectBuffer

Mike, I think I'd slightly prefer using the --wrap flags so nothing else allocates using the standard allocators. Is there any reason not to do that?

Comment 4

[Mike Hommey [:glandium]]

One reason is that --wrap only works when the wrapped function is in an external library. mozglue/android and jemalloc are in the same library.
The other reason is in comment 1.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Mike Hommey [:glandium] from comment #4)
> One reason is that --wrap only works when the wrapped function is in an
> external library. mozglue/android and jemalloc are in the same library.
> The other reason is in comment 1.

Right, I failed to read comment 1 before commenting. So, ignore comment 3.

Comment 6

[Mike Hommey [:glandium]]

Not that eventually, mozalloc will be folded in mozglue, at which point using operator new and delete in mozglue will use jemalloc. When that happens, we can remove this hack and replace malloc and free with new and delete.

Comment 7

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #6)
> Not that eventually,(...)

Note, not not

Comment 8

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e90fe61000f

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7e90fe61000f