test_minidom non-failure failure? (take 2)
Sorry about the previous message; a mail munger somewhere between my display and python.org choked on a very long line... I am getting an occasional, hard-to-reproduce error in test_minidom. When I run the test, it displays about a thousand lines of garbage, but the test suite does not report test_minidom as failed or skipped. The output I see during the test run is this: test_minidom garbage: [{'nodeValue': u'Obsolete but implemented...', 'nextSibling': <DOM Text node "\012">, 'childNodes': None, 'attributes': None, 'parentNode': None, 'data': u'Obsolete but implemented...', 'previousSibling': None}, <DOM Text node "Obsolete b...">, {'nodeValue': u'\012', 'nextSibling': None, 'childNodes': None, 'a [... many hundreds of lines omitted] At the end of the test, I get a pretty normal result: 95 tests OK. 13 tests skipped: test_al test_cd test_cl test_dbm test_dl test_gl test_imgfile test_largefile test_nis test_sunaudiodev test_timing test_winreg test_winsound So two questions: Why is test_minidom producing all this output? And why is it only happening intermittently? Why does regrtest.py think that test_minidom is working correctly when it produces all this output? Jeremy
On Thu, Oct 12, 2000 at 12:11:53PM -0400, Jeremy Hylton wrote:
I am getting an occasional, hard-to-reproduce error in test_minidom. When I run the test, it displays about a thousand lines of garbage, but the test suite does not report test_minidom as failed or skipped.
The output I see during the test run is this:
test_minidom garbage: [{'nodeValue': u'Obsolete but implemented...', 'nextSibling':
This is most likely the garbage collector. regrtest.py contains the following code: if findleaks: gc.collect() if gc.garbage: print "garbage:", repr(gc.garbage) found_garbage.extend(gc.garbage) del gc.garbage[:] findleaks is true if the -l option is specified (TESTOPS in the makefile includes it). Something is producing cyclic garbage. Neil
Neil Schemenauer writes:
This is most likely the garbage collector. regrtest.py contains the following code: ... findleaks is true if the -l option is specified (TESTOPS in the makefile includes it). Something is producing cyclic garbage.
This is definately the problem. Lars, Paul: This looks like a problem in the unlink() method of the DOM. Could you please check that the unlink() method is updated to handle the latest version of the other changes? Thanks! -Fred -- Fred L. Drake, Jr. <fdrake at beopen.com> BeOpen PythonLabs Team Member
* Fred L. Drake, Jr. | | This looks like a problem in the unlink() method of the DOM. Could | you please check that the unlink() method is updated to handle the | latest version of the other changes? It seems that the current unlink() does not remove sibling cycles. Patch #101897 adds a line to set sibling references to None, which seems to make regrtest.py -l happy. --Lars M.
Right, I just checked in the fix to that. Paul Prescod On 13 Oct 2000, Lars Marius Garshol wrote:
* Fred L. Drake, Jr. | | This looks like a problem in the unlink() method of the DOM. Could | you please check that the unlink() method is updated to handle the | latest version of the other changes?
It seems that the current unlink() does not remove sibling cycles. Patch #101897 adds a line to set sibling references to None, which seems to make regrtest.py -l happy.
--Lars M.
_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://www.python.org/mailman/listinfo/python-dev
On Thu, Oct 12, 2000 at 12:11:53PM -0400, Jeremy Hylton wrote:
I am getting an occasional, hard-to-reproduce error in test_minidom. When I run the test, it displays about a thousand lines of garbage, but the test suite does not report test_minidom as failed or skipped.
The output I see during the test run is this:
test_minidom garbage: [{'nodeValue': u'Obsolete but implemented...', 'nextSibling':
[Neil]
This is most likely the garbage collector. regrtest.py contains the following code:
if findleaks: gc.collect() if gc.garbage: print "garbage:", repr(gc.garbage) found_garbage.extend(gc.garbage) del gc.garbage[:]
findleaks is true if the -l option is specified (TESTOPS in the makefile includes it). Something is producing cyclic garbage.
Of course something is producing cyclic garbage! The DOM tree is full of parent and child links. Does this output mean that the GC works correctly? Or does it mean that there is a reason why this garbage cannot be disposed of? In the latter case, could that be because there are __del__ methods? --Guido van Rossum (home page: http://www.python.org/~guido/)
On Thu, Oct 12, 2000 at 12:39:40PM -0500, Guido van Rossum wrote:
Of course something is producing cyclic garbage!
The DOM tree is full of parent and child links.
Does this output mean that the GC works correctly? Or does it mean that there is a reason why this garbage cannot be disposed of? In the latter case, could that be because there are __del__ methods?
The -l option tries to find any cyclic garbage produced by the tests. I don't think that that option should be enabled default. The output means that the GC is working and is finding stuff that would not be freed by reference counting alone. I can't tell if the GC would free this garbage. The -l option sets the DEBUG_SAVEALL option which causes all garbage found to end up in gc.garbage, not just garbage the can't be cleaned up. I don't have pyexpat installed here so I can't test it. If you want to find out if test_minidom is creating garbage the collector can't free you should comment out the: gc.set_debug(gc.DEBUG_SAVEALL) line in regrtest.py and run: regrtest.py -l test_minidom If that does what I think it does and you still get the "garbage: " line then the test is creating evil things. :) Neil
"NS" == Neil Schemenauer <nas@arctrix.com> writes:
NS> The -l option tries to find any cyclic garbage produced by the NS> tests. I don't think that that option should be enabled NS> default. The output means that the GC is working and is finding NS> stuff that would not be freed by reference counting alone. NS> I can't tell if the GC would free this garbage. The -l option NS> sets the DEBUG_SAVEALL option which causes all garbage found to NS> end up in gc.garbage, not just garbage the can't be cleaned up. NS> I don't have pyexpat installed here so I can't test it. If you NS> want to find out if test_minidom is creating garbage the NS> collector can't free you should comment out the: NS> gc.set_debug(gc.DEBUG_SAVEALL) NS> line in regrtest.py and run: NS> regrtest.py -l test_minidom NS> If that does what I think it does and you still get the NS> "garbage: " line then the test is creating evil things. :) The test is not creating evil things. I commented out the DEBUG_SAVEALL line and got no error report. The question, then, is what to do about the -l option. I assume we should remove the -l option from the Makefile, so that "make test" doesn't turn on DEBUG_SAVEALL. Or do we need to change regrtest in some way so that it still reports on tests that create evil things? Jeremy
On Thu, Oct 12, 2000 at 04:10:50PM -0400, Jeremy Hylton wrote:
The question, then, is what to do about the -l option. I assume we should remove the -l option from the Makefile, so that "make test" doesn't turn on DEBUG_SAVEALL. Or do we need to change regrtest in some way so that it still reports on tests that create evil things?
This is a policy decision. Is it okay for the test suite to create garbage that is not collectable by reference counting? If yes, what about garbage that is not collectable by the (current) GC? Neil
This is a policy decision. Is it okay for the test suite to create garbage that is not collectable by reference counting?
I don't see why that should be forbidden. After all some of the code we test has such leaks -- we haven't declared those absolute bugs.
If yes, what about garbage that is not collectable by the (current) GC?
Can you give an example of how such garbage can be created? --Guido van Rossum (home page: http://www.python.org/~guido/)
On Thu, Oct 12, 2000 at 04:26:49PM -0500, Guido van Rossum wrote:
This is a policy decision. Is it okay for the test suite to create garbage that is not collectable by reference counting?
I don't see why that should be forbidden. After all some of the code we test has such leaks -- we haven't declared those absolute bugs.
Again, "-l" should probably not be a default. I don't know who added it to TESTOPTS but it wasn't me.
Can you give an example of how such garbage can be created?
Look at test_gc. Here is an example: class Foo: def __del__(self): pass foo = Foo() foo.foo = foo del foo Theoretically this structure could be collected without problem but the GC is too simple minded to realize that there is only one finalizer involved. Here's a better example: foo = Foo() bar = Foo() foo.bar = bar bar.foo = foo del foo, bar The GC cannot safely break this cycle so it punts and adds the instance objects involved to gc.garbage and forgets about it. Neil
"NS" == Neil Schemenauer <nas@arctrix.com> writes:
NS> Again, "-l" should probably not be a default. I don't know NS> who added it to TESTOPTS but it wasn't me. I did, but at the time -l set LEAK_DEBUG not DEBUG_SAVEALL, and I think (but don't really remember) that LEAK_DEBUG had different semantics than it does now. No matter. This was useful when gc was spankin' new and only semi-tested. It's probably fine to turn off -l in "make test" by default for the 2.0 release. -Barry
On Fri, Oct 13, 2000 at 01:33:57AM -0400, Barry A. Warsaw wrote:
I think (but don't really remember) that LEAK_DEBUG had different semantics than it does now.
Nope. LEAK_DEBUG prints to stderr information about all garbage the GC finds, even stuff the GC can free. The thinking was that GC would be an option and people would want to find applications that leak when only using reference counting.
No matter. This was useful when gc was spankin' new and only semi-tested. It's probably fine to turn off -l in "make test" by default for the 2.0 release.
I've left -l enabled for now. Detecting tests that create uncollectable garbage is probably a good thing. It doesn't cost much and the message should be fairly clear now. Neil
Neil Schemenauer writes:
I've left -l enabled for now. Detecting tests that create uncollectable garbage is probably a good thing. It doesn't cost much and the message should be fairly clear now.
Perhaps one thing that could be done is to reduce the volume of the output; perhaps just list the first 1K or 2K of the repr? -Fred -- Fred L. Drake, Jr. <fdrake at beopen.com> BeOpen PythonLabs Team Member
Neil Schemenauer writes:
It just prints the number of object found now.
Ok, I must have missed the checkin that changed that. This is fine in my book as well. Thanks for staying on top of this work! I think you've really done a great job with the GC effort. -Fred -- Fred L. Drake, Jr. <fdrake at beopen.com> BeOpen PythonLabs Team Member
The question, then, is what to do about the -l option. I assume we should remove the -l option from the Makefile, so that "make test" doesn't turn on DEBUG_SAVEALL.
This seems to make sense, yes. Since the "leaked" objects that are found will be released by the GC code, I see no point in reporting these during the regression test. This can only confuse people (like it did Jeremy :-).
Or do we need to change regrtest in some way so that it still reports on tests that create evil things?
Any form of evilness that can be detected without *too* much effort is worth it... I have no idea what kind of evil we're looking for here or how to detect is, so I can't answer yes or no. --Guido van Rossum (home page: http://www.python.org/~guido/)
On Thu, Oct 12, 2000 at 04:21:39PM -0500, Guido van Rossum wrote:
Any form of evilness that can be detected without *too* much effort is worth it... I have no idea what kind of evil we're looking for here or how to detect is, so I can't answer yes or no.
That would be reference cycles with finalizers (ie. instances with __del__ methods). The current garbage collector does not free such structures but instead appends them to gc.garbage. Sorry for not being clear. In any case, regrtest should probably print a more clueful message when objects are found in gc.garbage. That would go a long way in clearing up the confusion. I'll see about a patch. Neil
"NS" == Neil Schemenauer <nas@arctrix.com> writes:
NS> In any case, regrtest should probably print a more clueful NS> message when objects are found in gc.garbage. That would go a NS> long way in clearing up the confusion. I'll see about a patch. I think there is some value to issueing reports when a test creates trash that can't be cleaned up by the GC, but it doesn't make sense to report this by default. If "make test" reports an error, it should mean that something has gone wrong with the user's build and the interpreter is broken. That's not the case here. The build was fine; it's just the test that is iffy. I think we should turn of the -l option by default *and* patch it so that the output is more useful. I would suggest printing a count of the objects, perhaps organized by type/class name. Something like this: Test created 6 uncollectable objects 4 foo.Bar instances 2 foo.Baz instances Jeremy
Jeremy Hylton writes:
Why is test_minidom producing all this output? And why is it only happening intermittently?
It isn't. See Neil's excellent explanation.
Why does regrtest.py think that test_minidom is working correctly when it produces all this output?
The test is passing just fine, and is complete before the test for garbage is performed. The unlink() method on DOM objects is the culprit; it is updating the Node.allnodes dictionary correctly, but not the Node instances. I've already asked Paul & Lars to fix this; it should work just fine with or without GC once they've seen the report. -Fred -- Fred L. Drake, Jr. <fdrake at beopen.com> BeOpen PythonLabs Team Member
participants (7)
-
bwarsaw＠beopen.com -
Fred L. Drake, Jr. -
Guido van Rossum -
Jeremy Hylton -
Lars Marius Garshol -
Neil Schemenauer -
Paul