Ticket #898 (closed bug: fixed)

Opened 12 years ago

Last modified 11 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 12 years ago by jkeenan

For reference: r40313.

  Changed 12 years ago by coke

  • component changed from pmc2c to core

in reply to: ↑ description   Changed 11 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 11 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 11 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 11 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.