Closed Bug 1142412 Opened 5 years ago Closed 4 years ago

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

Categories

(Core :: Memory Allocator, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(3 files, 1 obsolete file)

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.
Looks like we're waiting for an upstream fix: https://github.com/jemalloc/jemalloc/issues/213
For patch readability, this doesn't actually include the jemalloc update, which will be in a separate patch.
Assignee: nobody → mh+mozilla
Attachment #8646769 - Flags: review?(n.nethercote)
Attached patch Apply update.shSplinter Review
Attachment #8646770 - Flags: review?(n.nethercote)
Updated configure.in to actually work properly. Turns out tast minute fixups don't always work.
Attachment #8646769 - Attachment is obsolete: true
Attachment #8646769 - Flags: review?(n.nethercote)
Attachment #8646853 - Flags: review?(n.nethercote)
Comment on attachment 8646853 [details] [diff] [review]
Update memory/jemalloc to current dev tip (1f27abc)

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

rs=me
Attachment #8646853 - Flags: review?(n.nethercote) → review+
Comment on attachment 8646770 [details] [diff] [review]
Apply update.sh

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

rs=me
Attachment #8646770 - Flags: review?(n.nethercote) → review+
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.
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 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.
Attachment #8647264 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/c94b259b603e
https://hg.mozilla.org/mozilla-central/rev/def7cf4cfbcc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(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?
(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.
You need to log in before you can comment on or make changes to this bug.