Ticket #898 (closed bug: fixed)

Opened 5 years ago

Last modified 4 years ago

r40313 introduces a memory leak

Reported by: cotto Owned by: cotto
Priority: normal Milestone:
Component: core Version: 1.4.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

Parrot_Foo_get_vtable() returns a malloc'd struct that's not freed by this code. This needs to be fixed.

Change History

  Changed 5 years ago by jkeenan

For reference: r40313.

  Changed 5 years ago by coke

  • component changed from pmc2c to core

in reply to: ↑ description   Changed 4 years ago by jkeenan

Replying to cotto:

Parrot_Foo_get_vtable() returns a malloc'd struct that's not freed by this code. This needs to be fixed.

cotto,

Can you provide an illustration of this leak that someone could use as a starting point?

Thank you very much.

kid51

  Changed 4 years ago by cotto

The conditions to trigger this leak are pretty rare but it'd still be nice to fix it up. The leak can only happen when a dynpmc (Foo) extends another dynpmc (Bar), and when one of Foo's VTABLE functions calls SUPER. In that case the following code is generated, which will leak one VTABLE* per invocation of that function.

    /* from src/dynpmc/foo2.c; note that the returned value isn't freed. */
    INTVAL i = Parrot_Foo_get_vtable(interp)->get_integer(interp, _self);

This could cause significant leakage if any HLLs had dynpmcs that extended other dynpmcs and called SUPER, but in practice it only happens in that one dynpmc in src/dynpmcs, and that's only to test that this case works.

The tricky part is that pmc2c needs to turn the "SUPER()" in something like INTVAL i = SUPER(); into a snippet that gets the parent's vtable, calls a function, frees the vtable and assigns the value to i (or otherwise dtrt).

  Changed 4 years ago by chromatic

>  The tricky part is that pmc2c needs to turn the "SUPER()" in something
>  like INTVAL i = SUPER(); into a snippet that gets the parent's vtable,
>  calls a function, frees the vtable and assigns the value to i (or
>  otherwise dtrt).

... or we add a new function to fetch a const pointer to the existing 
(uncopied) vtable.

You modify it, you get to keep all of the broken pieces.

-- c

  Changed 4 years ago by cotto

  • status changed from new to closed
  • resolution set to fixed

Thanks for the suggestion. This is fixed (and the hackishness thereof documented) in 9df28c8. The fix could be more efficient, but this is a rare enough case that it's not worth optimizing.

Note: See TracTickets for help on using tickets.