If JS_NewArrayBufferWithContents fails, ArrayBufferBuilder::getArrayBuffer calls js_free(nullptr)

RESOLVED FIXED in mozilla33

Status

()

defect
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: smaug, Assigned: mz_mhs-ctb)

Tracking

({memory-leak})

29 Branch
mozilla33
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment, 1 obsolete attachment)

http://hg.mozilla.org/mozilla-central/rev/ebdd57580809 looks odd.
    1.11    JSObject* obj = JS_NewArrayBufferWithContents(aCx, mLength, mDataPtr);
    1.12 +  mDataPtr = nullptr;
    1.13 +  mLength = mCapacity = 0;
    1.14    if (!obj) {
    1.15 +    js_free(mDataPtr);
    1.16      return nullptr;
    1.17    }

We set mDataPtr to null and then if obj is null, try to free mDataPtr.
Yeah, it looks like this will leak whatever was in mDataPtr on failure.
Keywords: mlk
Whiteboard: [MemShrink]
Posted patch xhr-leak.patch (obsolete) — Splinter Review
Assignee: nobody → mz_mhs-ctb
Attachment #8448490 - Flags: review?(bugs)
Attachment #8448490 - Flags: review?(bugs) → review+
Attachment #8448490 - Attachment is obsolete: true
Attachment #8448764 - Flags: review+
Keywords: checkin-needed
Hi Michael, 

could you provide a try run for this checkin? Thanks!
Keywords: checkin-needed
Unfortunately no, don't have commit access.
Flags: needinfo?(cbook)
I'll do a try run today and set checkin needed when it is done.
Flags: needinfo?(cbook) → needinfo?(continuation)
Sorry this took so long, our test infrastructure was really backed up for some reason.  Try run is green.
Flags: needinfo?(continuation)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/650af90512e2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.