Lack of garbage-collectible packfiles can lead to memory leaks

[coke]

The attached script leaks and eventually PANICs out of memory.

[coke]

anonymous sub memory leak

[chromatic]

The good news is that it's not a leak in the "Oh goodness, Valgrind shows exactly where the problem is!" The bad news is that Valgrind doesn't report where the problem is. I suspect the refcounts for the contexts of the anonymous subs increment too often.

[bacek]

Coke,

can you retest this one? It's probably gone after context_pmc3 branch merge.

-- Backe

[coke]

On Thu, Sep 10, 2009 at 9:53 AM, Parrot <parrot-tickets@lists.parrot.org> wrote:
> #783: anon noop subs leak memory.
> -------------------+--------------------------------------------------------
>  Reporter:  coke   |        Type:  bug
>   Status:  new    |    Priority:  normal
> Milestone:         |   Component:  core
>  Version:  trunk  |    Severity:  medium
>  Keywords:         |        Lang:
>    Patch:         |    Platform:
> -------------------+--------------------------------------------------------
>
> Comment(by bacek):
>
>  Coke,
>
>  can you retest this one? It's probably gone after context_pmc3 branch
>  merge.
>
>  --
>  Backe
>
> --
> Ticket URL: <https://trac.parrot.org/parrot/ticket/783#comment:3>
> Parrot <https://trac.parrot.org/parrot/>
> Parrot Development
> _______________________________________________
> parrot-tickets mailing list
> parrot-tickets@lists.parrot.org
> http://lists.parrot.org/mailman/listinfo/parrot-tickets
>

Still a problem as of r41179.


-- 
Will "Coke" Coleda

[bacek]

Hello.

We need GCable Packfiles...

-- Bacek

[mj41]

Probaly also related to #706.

[jkeenan]

bacek, whiteknight:

Can we get an update on the status of this ticket?

Thank you very much.

kid51

[bacek]

Hello.

Main problem - imcc (or whatever PIR compiler is) creates a new Packfile for compiled sub. This packfile attached to interp and not destroyed until after full parrot shutdown. Without GCable Packfiles we can't go further with fixing this bug.

-- Bacek

[whiteknight]

Can we merge this ticket into something else, or rename it to indicate the full extent of the problem?

[whiteknight]

We have GCable packfiles now after the IMCC interface refactors and the new PackfileView PMC merge. However, because packfiles are not being properly marked, we have to GC register them, which leads to the same problem.

What we need to do is:

• Remove the call to Parrot_pmc_gc_register from Parrot_pf_get_packfile_pmc
• Build Parrot, find the resulting errors, and track down why the packfile was collected.
• Fix those errors

We're much closer to fixing this ticket now, but we aren't there yet.

[jkeenan]

Error output when Parrot_pmc_gc_register() is commented out

[jkeenan]

When I simply comment out Parrot_pmc_gc_register from Parrot_pf_get_packfile_pmc, I get the error output in the file I just attached.

diff --git a/src/packfile/api.c b/src/packfile/api.c
index e8a4906..9bd444b 100644
--- a/src/packfile/api.c
+++ b/src/packfile/api.c
@@ -1239,7 +1239,7 @@ Parrot_pf_get_packfile_pmc(PARROT_INTERP, ARGIN(PackFile *
 
     /* TODO: We shouldn't need to register this here. But, this is a cheap
              fix to make sure packfiles aren't getting collected prematurely */
-    Parrot_pmc_gc_register(interp, ptr);
+    /* Parrot_pmc_gc_register(interp, ptr); */
     return ptr;
 }

[whiteknight]

Yes, that's a good data-point. This is going to create GC errors, which are going to manifest differently on different systems (or even on the same systems, where memory layout changes). What we need is to track down what Packfiles are being prematurely collected, and then find a way to anchor them into the object graph.

[whiteknight]

This issue is going to be fixed with GC-able packfiles, which are high up on our TODO list of development priorities. Since this is basically just a TODO item, I'm going to close this ticket. There's no chance that we just forget about the underlying need.