Closed Bug 1021126 Opened 6 years ago Closed 5 years ago

Leak of |data| on failure in ArrayBufferObject::create()

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1037358

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, memory-leak, Whiteboard: [MemShrink:P3][CID 1213975])

Attachments

(1 file)

if (!data) {
        ...
        if (nbytes <= usableSlots * sizeof(Value)) {
            ...
        } else {
            // If we allocate here...
            data = AllocateArrayBufferContents(cx, nbytes);
            if (!data)
                return nullptr;
        }
    }

    ...

    Rooted<ArrayBufferObject*> obj(cx, NewBuiltinClassInstance<ArrayBufferObject>(cx, allocKind, newKind));
    // and allocating |obj| fails, we leak |data|
    if (!obj)
        return nullptr;

This is a little tricky because presumably we don't want to free |data| if it was passed in as an argument.
This is sort of a regression from Bug 979480, but it looks like ArrayBufferObject::create() just had a different set of leaks-on-failures before that, so it didn't really make the situation any worse.
Whiteboard: [MemShrink] → [MemShrink][CID 1213975]
Summary: Leak on failure in ArrayBufferObject::create() → Leak of |data| on failure in ArrayBufferObject::create()
I think I'll take this, as it could be the cause of bug 1021932.
Assignee: nobody → continuation
Hrmph.  Sadly this did not help with bug 1021932.
Whiteboard: [MemShrink][CID 1213975] → [MemShrink:P3][CID 1213975]
This is kind of ugly, but I guess it works.
Comment on attachment 8438614 [details] [diff] [review]
Free |data| on failure in ArrayBufferObject::create()

try runs: https://tbpl.mozilla.org/?tree=Try&rev=3b55b51fe964
ASan try runs: https://tbpl.mozilla.org/?tree=Try&rev=78f0fc1f001b

The dt failures are another patch in those pushes.
Attachment #8438614 - Flags: review?(sphink)
Comment on attachment 8438614 [details] [diff] [review]
Free |data| on failure in ArrayBufferObject::create()

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

Sorry for not taking this one.

But I'd prefer it to be done with ScopedFreePtr<void> allocData = nullptr; ... allocData = data; with an allocData.forget() just before obj->initialize(nbytes, data, OwnsData).
Attachment #8438614 - Flags: review?(sphink)
Fixed in http://hg.mozilla.org/mozilla-central/rev/c1ea647e6a70
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Thanks for fixing this!
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1037358
You need to log in before you can comment on or make changes to this bug.