Closed Bug 961795 Opened 11 years ago Closed 11 years ago

GenerationalGC: Valgrind reports memory leaks related to AllocateArrayBufferContents

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file)

In valgrind builds, several leaks are reported, all of which pass through AllocateArrayBufferContents, for example: ==60355== 118,191 bytes in 11 blocks are definitely lost in loss record 5,227 of 5,227 ==60355== at 0x4C26B43: calloc (vg_replace_malloc.c:593) ==60355== by 0x9FF1033: js::MallocProvider<js::ThreadSafeContext>::calloc_(unsigned long) (Utility.h:150) ==60355== by 0x9FF1123: AllocateArrayBufferContents(JSContext*, unsigned int, void*) (TypedArrayObject.cpp:249) ==60355== by 0x9FF12E6: js::ArrayBufferObject::allocateSlots(JSContext*, unsigned int, bool) (TypedArrayObject.cpp:276) ==60355== by 0x9FF141B: js::ArrayBufferObject::create(JSContext*, unsigned int, bool) (TypedArrayObject.cpp:648) ==60355== by 0xA005377: (anonymous namespace)::TypedArrayObjectTemplate<unsigned char>::fromLength(JSContext*, unsigned int) (TypedArrayObject.cpp:2537) ==60355== by 0xA00543C: JS_NewUint8Array(JSContext*, unsigned int) (TypedArrayObject.cpp:3542) ==60355== by 0x8D9664A: mozilla::dom::TextEncoder::Encode(JSContext*, JS::Handle<JSObject*>, nsAString_internal const&, bool, mozilla::ErrorResult&) (TypedArray.h:151) ==60355== by 0x8AD375F: mozilla::dom::TextEncoderBinding::encode(JSContext*, JS::Handle<JSObject*>, mozilla::dom::TextEncoder*, JSJitMethodCallArgs const&) (TextEncoder.h:61) ==60355== by 0x8AC8FFF: mozilla::dom::TextEncoderBinding::genericMethod(JSContext*, unsigned int, JS::Value*) (TextEncoderBinding.cpp:238) ==60355== by 0x9FA08DE: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:220) ==60355== by 0x9F9C9F8: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2609) Full log here: https://tbpl.mozilla.org/php/getParsedLog.php?id=33285700&tree=Try&full=1#error0
Oh, I'm seeing these stacks also in DMD. I was about to try writing a memory reporter for them as part of bug 961883, but if they're leaks I might not need to...
Attached patch typed-array-elements (deleted) — Splinter Review
So for arrays objects allocated in the nursery, we keep track of the array elements when they are malloc allocated in the hugeSlots set. We don't seem to do this for typed arrays though, and I guess this means that if we collect the nursery when it contains such a typed array, we will simply leak the elements. Here's a patch to fix this. This doesn't stop valgrind complaining though, although now it complains about memory allocated through js::jit::NewSlots(). I'm not sure why this didn't show up previously, but I think the patch is still good.
Assignee: nobody → jcoppeard
Attachment #8365184 - Flags: review?(terrence)
Comment on attachment 8365184 [details] [diff] [review] typed-array-elements Review of attachment 8365184 [details] [diff] [review]: ----------------------------------------------------------------- Ouch! Great catch and I really should have seen that. This is good for now, but we really need to find a more automated way of managing that set because it's going to be a continual thorn until we do. r=me
Attachment #8365184 - Flags: review?(terrence) → review+
The NewSlots allocation site is the same issue. Both of these sites should probably be using Nursery::allocateSlots to get this management for freeish. I did it this way instead of intercepting the allocator directly because of the way PJS is set up to allocate. I should revisit that.
Whiteboard: [leave open] → [leave open][MemShrink]
(In reply to Terrence Cole [:terrence] from comment #5) I can only see NewSlots() getting called while creating a call object, and in this case we go through JSObject::create(), which calls notifyInitialSlots() on the nursery. So I don't understand what's going wrong.
Jon, are you still getting complaints from Valgrind? I can't tell what the state of this bug is.
Whiteboard: [leave open][MemShrink] → [leave open][MemShrink:P2]
Yes, having fixed the first problem, I now get reports like this: ==60918== 21,248 bytes in 83 blocks are definitely lost in loss record 6,070 of 6,073 ==60918== at 0x4C28A49: malloc (vg_replace_malloc.c:270) ==60918== by 0x9EAA264: js::jit::NewSlots(JSRuntime*, unsigned int) (Utility.h:144) ==60918== by 0x1BF2A92F: ??? ==60918== by 0x2D75ADBF: ??? ==60918== by 0x7FEFFD9AF: ??? ==60918== by 0x2D79307F: ??? ==60918== { <insert_a_suppression_name_here> Memcheck:Leak fun:malloc fun:_ZN2js3jit8NewSlotsEP9JSRuntimej obj:* obj:* obj:* obj:* } ==60918== LEAK SUMMARY: ==60918== definitely lost: 235,008 bytes in 918 blocks ==60918== indirectly lost: 4,760 bytes in 148 blocks ==60918== possibly lost: 3,449 bytes in 61 blocks ==60918== still reachable: 266,055 bytes in 2,115 blocks ==60918== suppressed: 593,257 bytes in 6,898 blocks ==60918== Reachable blocks (those to which a pointer was found) are not shown. ==60918== To see them, rerun with: --leak-check=full --show-reachable=yes ==60918== ==60918== For counts of detected and suppressed errors, rerun with: -v ==60918== ERROR SUMMARY: 836 errors from 836 contexts (suppressed: 72 from 66) TEST-UNEXPECTED-FAIL | valgrind-test | valgrind found errors
Work on the remaining Valgrind signature is ongoing in 969012. I don't think we need this bug anymore.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [leave open][MemShrink:P2] → [MemShrink:P2]
Target Milestone: --- → mozilla29
No longer depends on: 969012
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: