1149399 - Leaking name16 in JS::ubi::SimpleEdgeVectorTracer?

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

// The simplest code is correct! The temporary SimpleEdge takes
        // ownership of name; if the append succeeds, the vector element
        // then takes ownership; if the append fails, then the temporary
        // retains it, and its destructor will free it.
        if (!vec->append(mozilla::Move(SimpleEdge(name16, Node(kind, *thingp))))) {
            okay = false;
            return;
        }

However, name16 is just a char16_t*, not some kind of RAII smart pointer, so it doesn't have a dtor that cleans up after itself.

Am I missing something?

Asan is reporting leaks pointing here in my heap snapshot try pushes. Do we not exercise this code path currently?

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Jim, see comment 0.

Comment 2

[Jim Blandy :jimb]

(In reply to Nick Fitzgerald [:fitzgen] from comment #0)
> // The simplest code is correct! The temporary SimpleEdge takes
>         // ownership of name; if the append succeeds, the vector element
>         // then takes ownership; if the append fails, then the temporary
>         // retains it, and its destructor will free it.
>         if (!vec->append(mozilla::Move(SimpleEdge(name16, Node(kind,
> *thingp))))) {
>             okay = false;
>             return;
>         }
> 
> However, name16 is just a char16_t*, not some kind of RAII smart pointer, so
> it doesn't have a dtor that cleans up after itself.

The SimpleEdge and Node constructors never fail, so SimpleEdge(name16, ...) is guaranteed to construct a SimpleEdge. SimpleEdge is documented to take ownership of the name:

https://hg.mozilla.org/mozilla-central/file/c20f8549d631/js/public/UbiNode.h#l441

and hence from that point forward the string is owned by some SimpleEdge, whose destructor will free it.

I'm curious about these ASan failures. I'll find you on IRC.