classification
Title: test_bsddb3 leaks references
Type: resource usage Stage: resolved
Components: Extension Modules Versions: Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: flox Nosy List: ezio.melotti, flox, jcea, pitrou
Priority: normal Keywords: patch

Created on 2010-01-29 18:42 by flox, last changed 2010-03-16 12:55 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
issue7808_bsddb3_refleak_v2.diff flox, 2010-02-09 10:10 Patch, apply to trunk
issue7808_bsddb3_refleak_v2TOv3.diff jcea, 2010-03-14 07:15
issue7808_bsddb3_refleak_v2TOv3_mod.diff flox, 2010-03-14 19:14 Patch, apply to trunk
Messages (13)
msg98523 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-01-29 18:45
Patch proposed to fix the refleaks.
msg99100 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-02-09 10:10
Patch updated for latest trunk.
msg100271 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-03-01 21:11
Fixed with r78563 and r78564.
msg100347 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2010-03-03 15:00
Florent, Could you explain the changes to the unittest?. I don't understand them.
msg100348 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-03-03 15:34
> Could you explain the changes to the unittest?

The reference to "self" in the hooks were preventing the GC of the test case, as far as I understand, because it creates a cycle.
When using weak references, there's no more dead cycles.
It is my own explanation, maybe it's not exactly what happens.

For the sys.traceback, I could not explain it better than the FAQ:
http://docs.python.org/faq/design.html#how-does-python-manage-memory

I may add a small comment in test_replication to explain the usage of weakref.
msg101033 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2010-03-14 00:08
I confirm the leaks in the C code. How do you find them?

I see your point with the weakrefs in the unittest, but I don't see why they are so important. We are creating a GC cycle, yes, but the GC will collect it eventually, beside the reference counters being fooled because the cycle. Are you using a tool to find cycles?

I am committing your patch upstream and I need to understand the rationale :).

Thanks!.
msg101034 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-03-14 00:56
The tool is called regrtest:

~ $ ./python -m test.regrtest -R 2:3: -uall test_bsddb3

Some devs and some buildbots hunt refleaks in the test suite.
Even if the test-related refleaks are not important, they may hide real refleaks in the source code.
msg101035 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2010-03-14 01:02
You need a debug build of Python to use -R:
./configure --with-pydebug && make

See also: http://www.python.org/dev/faq/#how-do-i-create-a-debug-build-of-python
msg101036 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-03-14 01:04
> I confirm the leaks in the C code. How do you find them?

I've done it manually, disabling some tests, and running "regrtest -R" repeatedly until I isolate the function which is responsible.
Then I studied the source code, looking for the missing DECREFs.
msg101041 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2010-03-14 07:15
I have spend a few hours trying to understand the issue deeply, and I have an easier to understand version. It is bigger, but explain the issue good enough to use in a production program.

Please, Florent, review. It pass the leak test.

I rather prefer this patch because the weak references were sort of magic, and this new code actually shows what you should use in production.

If you think the patch is OK, it will be integrated in pybsddb 4.8.4 and then backported for python 2.7.

I also solved an undesired coupling between Replication Manager and Base Replication testsuites.
msg101062 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-03-14 19:14
Jesús, your patch looks good. I prefer your approach.
My use of the weakrefs was an ugly workaround, I admit.

Maybe the "close()" method in the C module should be in charge of DECREFing the handlers and freeing memory. I did not look further in this direction.

Attached patch is quite the same as yours. I just simplified the "dummy()" handler.
msg101113 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2010-03-15 14:14
Patch up-ported to pybsddb 4.8.4.
msg101165 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2010-03-16 12:55
I have problem getting your changes working in python 3.x. Yes, pybsddb supports py3k.

The problem is line "sys.exc_traceback = sys.last_traceback = None", that throws the following error under python 3.x:

"""
File "/home/pybsddb/build/lib.solaris-2.10-i86pc-3.1/bsddb3/tests/test_compare.py", line 215, in verifyStderr
    sys.exc_info()[2] = sys.last_traceback = None
TypeError: 'tuple' object does not support item assignment
"""

The line is automatically translated by "2to3".

Any suggestion?.

I guess in 3.x we can change the line to "sys.exc_clear()", and forget about the "None" asignment. What do you think?.
History
Date User Action Args
2010-03-16 12:55:36jceasetmessages: + msg101165
2010-03-15 14:14:21jceasetmessages: + msg101113
2010-03-14 19:14:34floxsetfiles: + issue7808_bsddb3_refleak_v2TOv3_mod.diff

messages: + msg101062
2010-03-14 07:15:28jceasetfiles: + issue7808_bsddb3_refleak_v2TOv3.diff

messages: + msg101041
2010-03-14 01:04:03floxsetmessages: + msg101036
2010-03-14 01:02:58ezio.melottisetmessages: + msg101035
2010-03-14 00:56:44floxsetmessages: + msg101034
2010-03-14 00:08:24jceasetmessages: + msg101033
2010-03-03 15:34:09floxsetmessages: + msg100348
2010-03-03 15:00:31jceasetmessages: + msg100347
2010-03-01 21:11:02floxsetstatus: open -> closed
resolution: fixed
messages: + msg100271

stage: patch review -> resolved
2010-03-01 19:43:26floxsetassignee: flox
2010-02-09 14:36:47floxsettype: performance -> resource usage
2010-02-09 14:27:55floxsetnosy: + pitrou
2010-02-09 13:28:21ezio.melottisetnosy: + ezio.melotti
2010-02-09 10:10:06floxsetfiles: + issue7808_bsddb3_refleak_v2.diff

messages: + msg99100
2010-02-09 10:08:55floxsetfiles: - issue7808_bsddb3_refleak.diff
2010-01-29 19:25:43floxsetnosy: + jcea
2010-01-29 18:45:25floxsetfiles: + issue7808_bsddb3_refleak.diff
keywords: + patch
messages: + msg98523

stage: patch review
2010-01-29 18:42:57floxcreate