[Python-Dev] Re: Evil Trashcan and GC interaction

[Guido van Rossum]

----------
> Guido:
> > Well, it *also* abuses the ob_refcnt field.

Neil:
> My patch fixes that too (by abusing the gc_prev pointer to chain trash
> together).

I think I haven't seen that patch yet.

> > How about this wild idea (which Tim & I had simultaneously
> > yesterday): change the trashcan code to simply leave the
> > object in the GC generational list, and let the GC code
> > special-case objects with zero refcnt so that they are
> > eventually properly disposed?
> 
> That could probably work.  What happens when the GC is disabled?

The trashcan code is also disabled.  Better not create cycles *or*
deeply nested containers.  They are similar anyway. :-)

> There is insidious bug here.  Andrew helped me walk through it and I
> think we figured it out.  First here's the code to trigger it:
> 
>     import gc
> 
>     class Ouch:
>         def __del__(self):
>             for x in range(20):
>                 list([])
> 
>     def f():
>         gc.set_threshold(5)
>         while 1:
>             t = () # <-- here
>             for i in range(300):
>                 t = [t, Ouch()]
>     f()
> 
> The line marked with "here" is where things go wrong.  t used to refer
> to a long chain of [t, Ouch()].  The SETLOCAL macro in ceval calls
> Py_XDECREF(GETLOCAL(i)).  That starts the deallocation of the list
> structure.  Ouch.__del__ gets called can creates some more objects,
> triggering a collection.  The f frame's traverse gets called and tries
> to follow the pointer for the t local.  It points to memory that was
> freed by _PyTrash_destroy_chain.

Yes, Tim & I figured this out just before lunch, too. :-(

> Hmm, now that I think about it the GC is not needed to trigger the bug:
> 
>     import gc
>     gc.disable()
>     import sys
> 
>     class Ouch:
>         def __del__(self):
>             print f_frame.f_locals['t']
> 
>     def f():
>         global f_frame
>         f_frame = sys._getframe()
>         while 1:
>             t = ()
>             for i in range(300):
>                 t = [t, Ouch()]
> 
>     f()
> 
> I haven't figured out the correct solution yet.  I'm just giving a
> status update. :-)

This is my patch:

Index: ceval.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Python/ceval.c,v
retrieving revision 2.308
diff -c -r2.308 ceval.c
*** ceval.c	24 Mar 2002 19:25:00 -0000	2.308
--- ceval.c	28 Mar 2002 18:21:09 -0000
***************
*** 554,561 ****
  /* Local variable macros */
  
  #define GETLOCAL(i)	(fastlocals[i])
! #define SETLOCAL(i, value)	do { Py_XDECREF(GETLOCAL(i)); \
! 				     GETLOCAL(i) = value; } while (0)
  
  /* Start of code */
  
--- 554,562 ----
  /* Local variable macros */
  
  #define GETLOCAL(i)	(fastlocals[i])
! #define SETLOCAL(i, value)	do { PyObject *tmp = GETLOCAL(i); \
! 				     GETLOCAL(i) = value; \
!                                      Py_XDECREF(tmp); } while (0)
  
  /* Start of code */
  

--Guido van Rossum (home page: http://www.python.org/~guido/)