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

Status: RESOLVED DUPLICATE of bug 1037358

(Core :: JavaScript Engine, defect)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

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.

Comment 2

[Andrew McCreight [:mccr8]]

I think I'll take this, as it could be the cause of bug 1021932.

Comment 3

[Andrew McCreight [:mccr8]]

Hrmph.  Sadly this did not help with bug 1021932.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Free |data| on failure in ArrayBufferObject::create()

This is kind of ugly, but I guess it works.

Comment 5

[Andrew McCreight [:mccr8]]

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.

Comment 6

[Steve Fink [:sfink] [:s:]]

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).

Comment 7

[André Bargull [:anba]]

Fixed in http://hg.mozilla.org/mozilla-central/rev/c1ea647e6a70

Comment 8

[Andrew McCreight [:mccr8]]

Thanks for fixing this!