Ticket #783 (closed bug: invalid)

Opened 13 years ago

Last modified 10 years ago

Lack of garbage-collectible packfiles can lead to memory leaks

Reported by: coke Owned by: whiteknight
Priority: major Milestone:
Component: core Version: master
Severity: medium Keywords:
Cc: Language:
Patch status: Platform: all

Description

The attached script leaks and eventually PANICs out of memory.

Attachments

leak.pir Download (0.6 KB) - added by coke 13 years ago.
anonymous sub memory leak
Parrot_pmc_gc_register.errors.edited.txt.gz Download (1.4 KB) - added by jkeenan 11 years ago.
Error output when Parrot_pmc_gc_register() is commented out

Change History

Changed 13 years ago by coke

anonymous sub memory leak

Changed 13 years ago by 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.

Changed 13 years ago by jkeenan

  • component changed from none to core

Changed 12 years ago by bacek

Coke,

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

-- Backe

Changed 12 years ago by 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

Changed 12 years ago by bacek

Hello.

We need GCable Packfiles...

-- Bacek

Changed 11 years ago by coke

  • lang set to perl6
  • priority changed from normal to major

Changed 11 years ago by mj41

Probaly also related to #706.

Changed 11 years ago by whiteknight

  • owner set to whiteknight

Changed 11 years ago by jkeenan

bacek, whiteknight:

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

Thank you very much.

kid51

Changed 11 years ago by 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

Changed 11 years ago by whiteknight

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

Changed 11 years ago by jkeenan

  • summary changed from anon noop subs leak memory. to Lack of garbage-collectible packfiles can lead to memory leaks

Changed 11 years ago by whiteknight

  • status changed from new to assigned

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.

Changed 11 years ago by whiteknight

  • lang perl6 deleted
  • platform set to all
  • version changed from trunk to master

Changed 11 years ago by jkeenan

Error output when Parrot_pmc_gc_register() is commented out

Changed 11 years ago by 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;
 }

Changed 11 years ago by 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.

Changed 10 years ago by whiteknight

  • status changed from assigned to closed
  • resolution set to invalid

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.

Note: See TracTickets for help on using tickets.