classification
Title: New-style classes fail to cleanup attributes
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, andrea.corbellini, belopolsky, cburroughs, christian.heimes, georg.brandl, georg.brandl, glchapman, gregory.p.smith, jimjjewett, loewis, nascheme, ncoghlan, pitrou, python-dev, sbt, vstinner
Priority: normal Keywords: needs review, patch

Created on 2006-08-23 18:24 by belopolsky, last changed 2013-05-10 20:35 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
x.py belopolsky, 2008-01-31 05:59 demo file
gc-import.patch belopolsky, 2008-11-11 05:23 Patch against revision 67183 review
gcshutdown.patch pitrou, 2013-05-04 19:45 review
gcshutdown2.patch pitrou, 2013-05-05 11:23 review
better_shutdown.patch pitrou, 2013-05-08 00:31 review
Messages (31)
msg60983 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2006-08-23 18:24
> cat x.py
class X(object):
    def __init__(self, x):
        self.x = x
        print 'creating X(%r)' % x

    def __del__(self):
        print 'deleting X(%r)' % self.x

class A(object):
    x = X('new')

class B:
    x = X('old')
> python x.py
creating X('new')
creating X('old')
deleting X('old')

Python 2.4.2 (#2, Jan 13 2006, 12:00:38)
Python 2.6a0 (trunk:51513M, Aug 23 2006, 14:17:11)
msg60984 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2006-08-23 22:01
Logged In: YES 
user_id=835142

It looks like the class object is not deleted alltogether:



class X(object):
    def __init__(self, x):
        self.x = x
        print 'creating X(%r)' % x

    def __del__(self):
        print 'deleting X(%r)' % self.x
        
class A(object):
    x = X('new')

del A

Output:
creating X('new')
deleting X('new')

msg60985 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-08-23 22:45
Logged In: YES 
user_id=849994

Note that new-style classes are always part of a reference
cycle (you can find this out via gc.get_referrers).
Therefore, they will not be deleted instantly, but only
after gc collects them (you can trigger that via gc.collect).
msg60986 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2006-08-23 22:54
Logged In: YES 
user_id=835142

Yes, I've found that (using gc.get_referrers!), but this
does not explain why A is not cleaned up when the program exits.

Note that if I put class A definition inside a function, it
does get cleaned up.  Must be some funny interation between
module and new-style class objects.
msg60987 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-08-23 23:18
Logged In: YES 
user_id=849994

There's also this sentence in the __del__ docs:
"""
It is not guaranteed that __del__() methods are called for
objects that still exist when the interpreter exits.
"""
msg60988 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2006-08-24 00:08
Logged In: YES 
user_id=835142

I used __del__ just to illustrate the problem. In real life
application, X was a type defined in a C module and I've
noticed that it's tp_dealloc is not called on instances
assigned to class variables.

BTW, what are the circumstances when __del__() methods are
not called for objects that still exist when the interpreter
exits?
msg60989 - (view) Author: Jim Jewett (jimjjewett) Date: 2006-08-30 13:22
Logged In: YES 
user_id=764593

The funny interaction with modules is probably that the 
module retains a reference to the class (and vice versa), 
so the class can't go away until the module does -- and a 
module in sys.modules can't go away.

The __del__ methods are not called if the interpreter can't 
decide which to call first.  For example, if

    A.attr=B
    B.attr=A

then A and B form a cycle (like the class and its defining 
module).  If only one has a __del__ method, it gets called, 
but if both do, then python doesn't know which to call 
first, so it never calls either.

You may have a cycle like

module <==> class <==>instanceA
               \  <==>instanceB

So that it can't decide whether to take care of instanceA 
or instanceB first.

Or it might be that the __del__ methods actually are being 
called, but not until module teardown has begun, so they 
don't work right.
msg60990 - (view) Author: Jim Jewett (jimjjewett) Date: 2006-08-30 13:34
Logged In: YES 
user_id=764593

Looking at your example code (as best I can guess about 
indentation), it looks like

module x <==> class X
module x <==> class A ==> A's instance x ==> class X
module x <==> class B ==> B's instance x ==> class X

So the x instances can't go away until A and B do, which 
means at module cleanup.  But when the module cleans up, it 
may well clean up X before A, so that A.x no longer has an 
active class, so that it can't find its __del__ method.

msg60991 - (view) Author: Jim Jewett (jimjjewett) Date: 2006-08-30 13:36
Logged In: YES 
user_id=764593

I suggest changing status to Pending Close - not a bug.
msg60992 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2006-08-30 13:53
Logged In: YES 
user_id=835142

Is it true that a class retains reference to the module?  The '__module__' attribute 
is a string, not a reference to the module.  Maybe you are talking about 
something else ...
msg60993 - (view) Author: Jim Jewett (jimjjewett) Date: 2006-08-30 14:30
Logged In: YES 
user_id=764593

More precisely, it retains a reference to the module's 
__dict__ as its globals.  At the moment, I'm not finding 
proof that this happens directly in the class, but it does 
happen in class methods -- including __del__.
msg61357 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-01-20 19:54
In the light of no further results, closing this bug.
msg61886 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-01-31 05:59
The problem still exists in 2.5.1.

The explanations given so far are not correct. With x.py as before (see 
attached):

>>> import sys, gc, x
creating X('new')
creating X('old')
>>> del x,sys.modules['x']
deleting X('old')
>>> gc.collect()
deleting X('new')
6

which shows that the cycles in x module are resolvable by GC.

The problem is not that there are uncollectable objects but that GC is 
ran on exit before x becomes dead.

>>> import sys, gc, x
creating X('new')
creating X('old')
>>> gc.set_debug(1)
>>> sys.exit()
gc: collecting generation 2...
gc: objects in each generation: 463 2034 0
gc: done.
deleting X('old')


Looking at the comments in Py_Finalize, it looks like GvR intended to 
run GC after destroying the modules, but it led to problems:
(from svn blame Python/pythonrun.c)
 32278 gvanrossum 
  9025      guido       /* Destroy all modules */
  8403      guido       PyImport_Cleanup();
  9025      guido 
 32278 gvanrossum       /* Collect final garbage.  This disposes of 
cycles created by
 34776    tim_one        * new-style class definitions, for example.
 34776    tim_one        * XXX This is disabled because it caused too 
many problems.  If
 34776    tim_one        * XXX a __del__ or weakref callback triggers 
here, Python code has
 34776    tim_one        * XXX a hard time running, because even the sys 
module has been
 34776    tim_one        * XXX cleared out (sys.stdout is gone, 
sys.excepthook is gone, etc).
 34776    tim_one        * XXX One symptom is a sequence of information-
free messages
 34776    tim_one        * XXX coming from threads (if a __del__ or 
callback is invoked,
 34776    tim_one        * XXX other threads can execute too, and any 
exception they encounter
 34776    tim_one        * XXX triggers a comedy of errors as subsystem 
after subsystem
 34776    tim_one        * XXX fails to find what it *expects* to find 
in sys to help report
 34776    tim_one        * XXX the exception and consequent unexpected 
failures).  I've also
 34776    tim_one        * XXX seen segfaults then, after adding print 
statements to the
 34776    tim_one        * XXX Python code getting called.
 34776    tim_one        */
 34776    tim_one #if 0
 32278 gvanrossum       PyGC_Collect();
 34776    tim_one #endif

Commenting out PyGC_Collect() seems like a too radical solution because  
no module referenced cycles get collected, not even those without 
__del__.

I have not tried it yet, but it looks like a possible solution is to 
call PyGC_Collect() at the end of _PyModule_Clear.
msg61889 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-01-31 08:37
In PyImport_Cleanup(), sys and __builtin__ are the last ones deleted.
What if PyGC_Collect() is called just before?
msg75732 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-11-11 05:23
amaury> What if PyGC_Collect() is called just before?

That would work.  With the following patch:

===================================================================
--- Python/import.c	(revision 67183)
+++ Python/import.c	(working copy)
@@ -498,7 +498,10 @@
 			PyDict_SetItem(modules, key, Py_None);
 		}
 	}
-
+	/* Collect garbage remaining after deleting the
+	   modules. Mostly reference cycles created by new style
+	   classes. */
+ 	PyGC_Collect();
 	/* Next, delete sys and __builtin__ (in that order) */
 	value = PyDict_GetItemString(modules, "sys");
 	if (value != NULL && PyModule_Check(value)) {

$ ./python.exe x.py
creating X('new')
creating X('old')
deleting X('old')
deleting X('new')
msg114273 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-18 20:01
Reopened because the history shows comments and patches months after it was set to closed and won't fix, see msg61886.
msg114274 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-08-18 20:09
This is superseded by issue812369, but as a stop-gap measure, I don't see any downside of applying gc-import.patch.
msg118213 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-10-08 18:49
Does anyone want to weigh in on this?  I am merging in the issue812369 nosy list.

I would like to either apply gc-import.patch or close this as superseded by issue812369.
msg118244 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2010-10-09 04:49
looks harmless to me.  though i think issue812369 looks okay as well at first glance.
msg188400 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-04 19:45
Here is a patch adding a call to gc.collect() after cleaning up most modules, with tests.
msg188435 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-05 11:23
Here is an updated patch after the latest changes on default.
msg188569 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-06 19:17
New changeset f0833e6ff2d2 by Antoine Pitrou in branch 'default':
Issue #1545463: Global variables caught in reference cycles are now garbage-collected at shutdown.
http://hg.python.org/cpython/rev/f0833e6ff2d2
msg188570 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-06 19:20
This issue is endly fixed in 3.4. Since changing the shutdown sequence is a delicate change, I won't backport to bugfix branches.
msg188651 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-05-07 13:44
The test seems to be failing on Windows.
msg188664 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-07 15:20
> The test seems to be failing on Windows.

Yes. I'll try to setup a new Windows dev environment and take a look :-/
msg188678 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-05-07 18:42
I think the problem is that the __del__ method fails on Windows, maybe because sys.stdout and sys.__stderr__ have been replaced by None.

Consider the following program:

  import os

  class C:
      def __del__(self, write=os.write):
          write(1, b"BEFORE\n")
          print("__del__ called")
          write(1, b"AFTER\n")

  l = [C()]
  l.append(l)

On Unix I get

  BEFORE
  __del__ called
  AFTER

but on Windows I only get

  BEFORE

I would suggest using os.write() instead of print() in the tests.
msg188679 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-05-07 18:45
I will try a fix.
msg188681 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-05-07 19:14
On Windows my encoding for stdout, stderr is "cp1252" which is implemented in pure python.

By the time that _PyGC_DumpShutdownStats() runs the encoding.cp1252 module has been purged so stdout and stderr are broken.

I am afraid I will have to leave this to you Antoine...
msg188690 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-07 22:29
Your diagnosis seems right about test_garbage_at_shudown
(I can reproduce under Linux using `PYTHONIOENCODING=iso8859-15 ./python -m test -v test_gc`).
msg188697 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-08 00:31
Here is a patch, it seems to work on the custom buildbots. The problem was two-fold:
- PyErr_Warn() is too high-level, it will invoke linecache and others
- encodings and codecs shouldn't be cleared before the final shutdown
msg188714 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-08 11:23
New changeset 8a5bebea9fec by Antoine Pitrou in branch 'default':
Issue #1545463: At shutdown, defer finalization of codec modules so that stderr remains usable.
http://hg.python.org/cpython/rev/8a5bebea9fec
History
Date User Action Args
2013-05-10 20:35:25pitrousetstatus: open -> closed
assignee: belopolsky ->
2013-05-08 11:23:34python-devsetmessages: + msg188714
2013-05-08 00:31:06pitrousetfiles: + better_shutdown.patch

messages: + msg188697
2013-05-07 22:36:52vstinnersetnosy: + vstinner
2013-05-07 22:29:11pitrousetmessages: + msg188690
2013-05-07 19:14:46sbtsetmessages: + msg188681
2013-05-07 18:45:46sbtsetmessages: + msg188679
2013-05-07 18:42:57sbtsetmessages: + msg188678
2013-05-07 15:20:56pitrousetmessages: + msg188664
2013-05-07 13:44:58sbtsetstatus: closed -> open
nosy: + sbt
messages: + msg188651

2013-05-06 19:20:10pitrousetstatus: open -> closed
resolution: fixed
messages: + msg188570

stage: patch review -> resolved
2013-05-06 19:17:20python-devsetnosy: + python-dev
messages: + msg188569
2013-05-05 11:23:17pitrousetfiles: + gcshutdown2.patch

messages: + msg188435
2013-05-05 09:18:06arigosetnosy: - arigo
2013-05-04 19:45:33pitrousetfiles: + gcshutdown.patch

type: resource usage -> enhancement
versions: + Python 3.4, - Python 3.2
keywords: + patch
nosy: + ncoghlan

messages: + msg188400
stage: commit review -> patch review
2012-11-17 17:49:46brett.cannonsetnosy: - brett.cannon
2010-10-09 04:49:38gregory.p.smithsetmessages: + msg118244
2010-10-08 18:49:14belopolskysetversions: + Python 3.2, - Python 2.6, Python 2.5, Python 2.7
nosy: + loewis, brett.cannon, arigo, nascheme, glchapman, gregory.p.smith, pitrou, christian.heimes, andrea.corbellini, cburroughs, - BreamoreBoy

messages: + msg118213

keywords: + needs review, - patch
stage: commit review
2010-08-24 23:03:17facundobatistasetversions: - Python 2.5.3
2010-08-18 20:09:00belopolskysetassignee: belopolsky
messages: + msg114274
2010-08-18 20:01:50BreamoreBoysetstatus: closed -> open

messages: + msg114273
resolution: wont fix -> (no value)
nosy: + BreamoreBoy
2008-11-11 05:25:15belopolskysetversions: + Python 2.6, Python 2.7, Python 2.5.3
2008-11-11 05:23:54belopolskysetfiles: + gc-import.patch
keywords: + patch
messages: + msg75732
2008-01-31 08:37:35amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg61889
2008-01-31 05:59:29belopolskysetfiles: + x.py
type: resource usage
messages: + msg61886
versions: + Python 2.5, - Python 2.4
2008-01-20 19:54:22georg.brandlsetstatus: open -> closed
nosy: + georg.brandl
resolution: wont fix
messages: + msg61357
2006-08-23 18:24:53belopolskycreate