Issue1582

classification
Title: com.ziclix.python.sql.PyConnection leaks memory
Type: behaviour Severity: normal
Components: zxjdbc Versions: 2.5.1, 2.5.0
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pjenvey Nosy List: akruis, pjenvey
Priority: Keywords: patch

Created on 2010-03-25.17:36:47 by akruis, last changed 2010-04-10.20:51:05 by pjenvey.

Files
File name Uploaded Description Edit Remove
PyConnection.diff akruis, 2010-03-25.17:36:45 patch for this issue: diff against rev 6988 of PyConnection.java
Messages
msg5593 (view) Author: Anselm Kruis (akruis) Date: 2010-03-25.17:36:44
Hi,

The zxjdbc PyConnection class contains two HashSets - 
"private Set<PyCursor> cursors;" and
"private Set<PyStatement> statements;"

I found the following issue with the "cursors" set:
The PyConnection#cursor() methods create PyCursor objects and add these objects to the "cursors" set. The PyCursor#close() method removes the cursors from the "cursors" set.

PEP249 defines the PyCurser#close() method: "Close the cursor now (rather than whenever __del__ is called)." This wording implies, that it is not required to explicitly close a cursor, because __del__ will close it anyway. 

Now, if one doesn't close a zxjdbc PyCursor manually and the cursor object becomes unreachable from python code, we get a memory leak, because the hard reference of the PyConnection to the PyCursor object prevents the finalisation of the cursor. That's a problem for long running database connections, which perform heavy batch processing. (I found this issue writing a batch import script for a django on jython powered site.)

Proposed solution:

I propose to change the current implementation of the Set-fields in 
PyConnection to become a WeakHashSet (actually a WeakHashMap using only the keySEt, because the java runtime lacks a WeakHashSet). This way, the PyConnection still can close all cursors in its PyConnection#close method, but doesn't prevent PyCursor from being finalized/garbage collected. 

During the development of the attached patch, I found the same issue probably exists with the "statements"-set. Therefore I made this set a weak set too. I verified the patch using the mat heap dump analyser.
msg5652 (view) Author: Philip Jenvey (pjenvey) Date: 2010-04-10.20:51:05
ok, fixed it to use a synchronized weak Set in r7015

thanks!
History
Date User Action Args
2010-04-10 20:51:05pjenveysetstatus: open -> closed
resolution: fixed
messages: + msg5652
2010-04-10 19:10:51pjenveysetassignee: pjenvey
nosy: + pjenvey
2010-03-25 17:36:47akruiscreate