sprhawk / python-on-a-chip

Automatically exported from code.google.com/p/python-on-a-chip
Other
0 stars 0 forks source link

GC collects local variables #92

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I found that defect in the following scenario, but 
it is a general design problem.

starting point:
the python code imports a module that is
larger than the free heap space but there 
is still 1K heap free.

scenario:

1. interp.c (Line 1475) calls 
   retval = mod_import(pobj1, &pobj2);

2. module.c (L 95) calls
   retval = obj_loadFromImg(memspace, &imgaddr, &pobj);

3. obj.c (L 77) calls
   retval = co_loadFromImg(memspace, paddr, r_pobj);

4. codeobj.c (L 46) calls
   retval = heap_getChunk(sizeof(PmCo_t), &pchunk);
   PM_RETURN_IF_ERROR(retval);
   pco = (pPmCo_t)pchunk;

5. codeobj.c (L 77) calls
   retval = obj_loadFromImg(memspace, paddr, &pobj);

6. obj.c (L67) calls
   retval = tuple_loadFromImg(memspace, paddr, r_pobj);

7. tuple.c (L 47) calls
   retval = tuple_new(n, r_ptuple);

8. tuple.c (L 80) calls
   /* Allocate a tuple */
   retval = heap_getChunk(size, (uint8_t **)r_ptuple);

Now there is not enough free heap space, and the garbage collector
is started in heap.c at line 481. The garbage collector does not mark
the local variable pco (see 4.), which means pco is collected, freed
and corrupted.

The raised error is a completly different one:

9. module.c (L 99) calls
   return mod_new((pPmObj_t)pco, pmod);

10. module.c (L 41) raises PM_RET_EX_TYPE
    because OBJ_GET_TYPE(pco) != OBJ_TYPE_COB 
    pco was corrupted by the garbage collector

Original issue reported on code.google.com by i...@moticon.de on 8 Jun 2010 at 11:31

GoogleCodeExporter commented 9 years ago
Thanks for the report.  Excellent detail.  This is the same as Issue #39.

This issue is actually a well-known problem of some kinds of garbage 
collectors.  I attempted to solve issue #39 once by using a temporary anchor 
(set the first new object as a temporary root), but that solution does not work 
well for the recursive function calls found in the obj_loadFromImg() call tree.

The next obvious solution is to create a list of temporary roots and append 
each new object until the final link occurs.  This is difficult for 2 reasons: 
creating a list to hold the temporary roots requires more RAM; finding all the 
places in the code where the anchor list needs to be made is not easy.

I am open to recommendations for fixing this.

Original comment by dwhall...@gmail.com on 8 Jun 2010 at 10:09

GoogleCodeExporter commented 9 years ago
How did you implement the temporary anchor?

Original comment by i...@moticon.de on 9 Jun 2010 at 8:46

GoogleCodeExporter commented 9 years ago
The attached patch file does not solve the issue.  But it does show how I tried 
to solve this issue using a temporary anchor.  The patch applies against r492.

Original comment by dwhall...@gmail.com on 9 Jun 2010 at 12:23

Attachments:

GoogleCodeExporter commented 9 years ago
The attached file is a proposal against r496.
The main idea is the following:

There is a fixed temporary root c array. 
Each heap_getChunkImpl saves the returned pointer in this array. 
The array is cleaned inside the interpreter loop. 

But there is some need for optimation: 
For example tuple_loadFromImg can create a variable size of elements,
causing an overflow in the c array. 
The paired functions heap_beginAlloc, heap_endAlloc, should avoid
an overflow.

Original comment by i...@moticon.de on 10 Jun 2010 at 5:21

Attachments:

GoogleCodeExporter commented 9 years ago
Michael thank you for your patches.  I knew an array of temp anchors was the 
way to go, but your code showed me how to push/pop the index into the array.  
That was the idea I was missing.  I've mainlined Issue #39, so you might want 
to check that it fixes your issue.  Please reply on the maillist or issue #39 
if there is any more trouble.

Original comment by dwhall...@gmail.com on 25 Jul 2010 at 10:40