1142412 - <jemalloc>: Error in VirtualFree(): Attempt to access invalid address

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Mike Hommey [:glandium]]

The error comes from:
https://dxr.mozilla.org/mozilla-central/source/memory/jemalloc/src/src/chunk_mmap.c#73

It is due to a code path that tries to unmap trailing pages when shrinking huge allocations. That is:
 foo = malloc(16 * 1024 * 1024);
 foo = realloc(foo, 10 * 1024 * 1024);
makes jemalloc want to pages_unmap the last 6MB of the allocation, but that's not possible on Windows, and that's what the error message is about. On jemalloc with --enable-debug, this also abort()s, so this turns several tests orange with the patch from bug 1142403 applied. It is worth noting, though, that if foo is further free()ed, the entire 16MB are correctly unmapped. That still wastes address space if we hold on to foo.

Comment 1

[Eric Rahm [:erahm]]

Looks like we're waiting for an upstream fix: https://github.com/jemalloc/jemalloc/issues/213

Comment 2

[Mike Hommey [:glandium]]

Attachment: Update memory/jemalloc to current dev tip (1f27abc)

For patch readability, this doesn't actually include the jemalloc update, which will be in a separate patch.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Apply update.sh

Comment 4

[Mike Hommey [:glandium]]

Attachment: Update memory/jemalloc to current dev tip (1f27abc)

Updated configure.in to actually work properly. Turns out tast minute fixups don't always work.

Comment 5

[Nicholas Nethercote [inactive]]

Comment on attachment 8646853 [details] [diff] [review]
Update memory/jemalloc to current dev tip (1f27abc)

Review of attachment 8646853 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 6

[Nicholas Nethercote [inactive]]

Comment on attachment 8646770 [details] [diff] [review]
Apply update.sh

Review of attachment 8646770 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 7

[Mike Hommey [:glandium]]

Attachment: Remove last jemalloc patch by using a workaround when running its configure

Comment 8

[Nicholas Nethercote [inactive]]

Comment on attachment 8647264 [details] [diff] [review]
Remove last jemalloc patch by using a workaround when running its configure

Review of attachment 8647264 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +9205,5 @@
>    if ! test -e memory/jemalloc; then
>      mkdir -p memory/jemalloc
>    fi
>  
> +  (PATH="$srcdir/memory/jemalloc/helper:$PATH";

A comment here pointing to the comment in memory/jemalloc/helper/git would be helpful. I had no idea what this was for until I read that comment.

Comment 9

[Nicholas Nethercote [inactive]]

The only downside of this is that next time someone has to introduce a local patch, they won't have an existing one to cargo-cult from :)

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8647264 [details] [diff] [review]
Remove last jemalloc patch by using a workaround when running its configure

Looks good, though I agree with #c8 that a comment about why the PATH change is there would be helpful.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c94b259b603e
https://hg.mozilla.org/integration/mozilla-inbound/rev/def7cf4cfbcc

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c94b259b603e
https://hg.mozilla.org/mozilla-central/rev/def7cf4cfbcc

Comment 13

[marvinhk]

(In reply to Eric Rahm [:erahm] from comment #1)
> Looks like we're waiting for an upstream fix:
> https://github.com/jemalloc/jemalloc/issues/213

just curious, this is fixed in v4.0.0 upstream, so we are going to have jemalloc3 or jemalloc4?

Comment 14

[Mike Hommey [:glandium]]

(In reply to marvinhk from comment #13)
> (In reply to Eric Rahm [:erahm] from comment #1)
> > Looks like we're waiting for an upstream fix:
> > https://github.com/jemalloc/jemalloc/issues/213
> 
> just curious, this is fixed in v4.0.0 upstream, so we are going to have
> jemalloc3 or jemalloc4?

What landed is jemalloc 4.