memory leaks (patches included)

[GoogleCodeExporter]

I use the new version "pymssql-2.0.0b1-dev-20130111.win32-py2.7.exe"
And Coding below:

############################
while True:
  conn = pymssql.connect(host='192.168.1.100', user='root', password='1234', database='db1', charset='utf8')
  cur = conn.cursor()
  cur.execute('select MAX(ID) from test')
  cur.close()
  conn.close()
  time.sleep(5)
############################
I run this program whole night, tomorrow i find the memory is out

Original issue reported on code.google.com by lclll.m...@gmail.com on 17 Apr 2013 at 7:24

[GoogleCodeExporter]

Not only windows, also on Linux in both pymssql and _mssql variants. For now I 
can see about ~40kb memleak with every query (simple query, 1 row result).

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 12:48

[GoogleCodeExporter]

Interesting, that nobody cares about memleak bug.

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 1:00

[GoogleCodeExporter]

i have to choice pyodbc ......

Original comment by lclll.m...@gmail.com on 20 Jun 2013 at 1:20

[GoogleCodeExporter]

https://groups.google.com/forum/?fromgroups#!topic/pymssql/tQP9yiRpo78

Little play with valgrind shows:

==21208== LEAK SUMMARY:
==21208==    definitely lost: 9,666,560 bytes in 1,180 blocks
==21208==    indirectly lost: 0 bytes in 0 blocks
==21208==      possibly lost: 983,124 bytes in 179 blocks
==21208==    still reachable: 3,282,382 bytes in 4,416 blocks
==21208==         suppressed: 0 bytes in 0 blocks
==21208== Reachable blocks (those to which a pointer was found) are not shown.

and example of leak track:

==21208== 4,841,472 bytes in 591 blocks are definitely lost in loss record 
1,194 of 1,194
==21208==    at 0x4C2C27B: malloc (in 
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==21208==    by 0x6AA8ACA: __pyx_tp_new_6_mssql_MSSQLConnection (_mssql.c:4172)
==21208==    by 0x4EDCDE2: ??? (in /usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4ECE40C: PyObject_Call (in /usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x6AA85E6: __pyx_pw_6_mssql_11connect (_mssql.c:16676)
==21208==    by 0x4EE860A: PyEval_EvalFrameEx (in 
/usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4EEB5C2: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4F19891: PyEval_EvalCode (in /usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4F26D70: ??? (in /usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4F27135: PyRun_FileExFlags (in /usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4F27A0C: PyRun_SimpleFileExFlags (in 
/usr/lib64/libpython2.7.so.1.0)
==21208==    by 0x4F3121C: Py_Main (in /usr/lib64/libpython2.7.so.1.0)

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 1:51

[GoogleCodeExporter]

lclll, you can use workaround with model of fork+dostuff+die, or 
exec+dostuff+die, e.g. fork, connect, query, die (or the same way with exec 
/subprocess/). Also can use object proxing with RPyC to use locally isolated 
process to do queries and die after some TTL.

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 1:53

[GoogleCodeExporter]

Can you change issue title? If so please remove "windows platform", bug affects 
also other platforms.

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 1:56

[GoogleCodeExporter]

sorry, i can't find how to change issue title, maybe you can create a new issus.

Original comment by lclll.m...@gmail.com on 20 Jun 2013 at 2:07

[GoogleCodeExporter]

Guys, can you try this patch (a shot in the dark by simple code inspction, I 
don't have a DB serber at hand fight now) and see it it solves the issue?:

diff --git a/_mssql.pyx b/_mssql.pyx
--- a/_mssql.pyx
+++ b/_mssql.pyx
@@ -581,6 +581,8 @@
             self.dbproc = NULL

         self._connected = 0
+        PyMem_Free(self.last_msg_proc)
+        PyMem_Free(self.last_msg_srv)
         PyMem_Free(self.last_msg_str)
         PyMem_Free(self._charset)
         connection_object_list.remove(self)

Original comment by cra...@gmail.com on 20 Jun 2013 at 2:49

[GoogleCodeExporter]

s/serber/server/

Original comment by cra...@gmail.com on 20 Jun 2013 at 2:50

[GoogleCodeExporter]

yea, found that already and it fixes a little, but there are still leaks:

==29645== LEAK SUMMARY:
==29645==    definitely lost: 0 bytes in 0 blocks
==29645==    indirectly lost: 0 bytes in 0 blocks
==29645==      possibly lost: 843,860 bytes in 162 blocks
==29645==    still reachable: 3,257,806 bytes in 4,413 blocks
==29645==         suppressed: 0 bytes in 0 blocks
==29645== Reachable blocks (those to which a pointer was found) are not shown.

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 2:52

[GoogleCodeExporter]

Thanks Jakub for the feedback. Were you able to run the pymssql test suite with 
the above changes to confirm they don't break anything else?

Are the remaning leaks locate also on the connection-establishing code? One way 
to check and isolate that would be to not issue any query, just simply connect 
and disconnect in a loop.

Also, if you could describe your setup to use Valgring + Cython would be very 
useful for further pymssql work. Is it necessary to re-compile Python (as per 
http://wiki.cython.org/UsingValgrindToDebug and 
http://hg.python.org/cpython/raw-file/tip/Misc/README.valgrind)?

Thanks again!

Original comment by cra...@gmail.com on 20 Jun 2013 at 3:02

[GoogleCodeExporter]

Can't confirm anything right now. Leak is still there, but above one is fixed I 
think.

Code works well for now if asking about SEGV after adding free() ;-)

Changed test to simple connect() and close() in loop:

##################
i=10
while i>0:
    i-=1
    print "*",i
    c=_mssql.connect(.....)
    c.cancel()
    c.close()
##################

Leak summary stays the same, value of i doesn't matter.

==392== LEAK SUMMARY:
==392==    definitely lost: 0 bytes in 0 blocks
==392==    indirectly lost: 0 bytes in 0 blocks
==392==      possibly lost: 842,804 bytes in 160 blocks
==392==    still reachable: 3,024,386 bytes in 4,363 blocks
==392==         suppressed: 0 bytes in 0 blocks

But still VmRSS grows constanly.

Original comment by jkolcza...@gmail.com on 20 Jun 2013 at 3:41

[GoogleCodeExporter]

Found main leak! Give me some time to prepare patch and test.

Original comment by jkolcza...@gmail.com on 21 Jun 2013 at 9:02

[GoogleCodeExporter]

Ok, may need a little help.

Main leak problem is in garbage collecting of mssql MSSQLConnection if still 
exists in active scope (e.g. object instance, i guess). When left as-is after 
.close() it's not garbage collected until leaving the scope, even if there is 
no reference to it anymore (like example above, variable c is assigned by new 
instance of MSSQLConnection, but old is not garbage collected until leaving the 
scope.

Fast workaround is to use explicitly del (adding "del c" after "c.close()" 
stops growing VmRSS.

So, for know, we have fully identified problem.

Original comment by jkolcza...@gmail.com on 21 Jun 2013 at 9:14

[GoogleCodeExporter]

Attaching patch for pymssql-2.0.0b1-dev-20130403 to fix leak caused by missing 
PyMem_Free (the same mentioned before).

Would be nice to add notice to documentation about garbage collecting problem 
and workaround (for now).

I hope lclll (I don't see names here, sorry) will be happy now ;-)

Original comment by jkolcza...@gmail.com on 21 Jun 2013 at 9:24

Attachments:

• _mssql.issue119.patch

[GoogleCodeExporter]

> Ok, may need a little help.

Main leak problem is in garbage collecting of mssql MSSQLConnection if still 
exists in active scope (e.g. object instance, i guess). When left as-is after 
.close() it's not garbage collected until leaving the scope, even if there is 
no reference to it anymore (like example above, variable c is assigned by new 
instance of MSSQLConnection, but old is not garbage collected until leaving the 
scope.

> Fast workaround is to use explicitly del (adding "del c" after "c.close()" 
stops growing VmRSS.

> So, for know, we have fully identified problem.

Wait, this could be (related to) Issue 107 () that has a proposed fix.

Original comment by cra...@gmail.com on 21 Jun 2013 at 2:14

[GoogleCodeExporter]

Maybe, or maybe not. Connection removes it self from connection_object_list in 
close(), so removes reference (my testing code uses close()). If that would be 
a problem, probably leaving active scope would not remove connection and it's 
resources (would not be garbage collected at scope leaving, cause of global 
reference in connection_object_list).

As far as I remember, I tested it by removing whole connection_object_list list 
(and references to it), cause I was checking if references are the problem. 
VmRSS was still growing. I can test it again with weakrefs.

Original comment by jkolcza...@gmail.com on 21 Jun 2013 at 3:23

[GoogleCodeExporter]

Original comment by msabr...@gmail.com on 6 Aug 2013 at 9:48

• Changed title: memory leaks (patches included)

[GoogleCodeExporter]

Thank you for the issue report and the patches. I will try to review these 
soon...

Original comment by msabr...@gmail.com on 6 Aug 2013 at 9:55

[GoogleCodeExporter]

Original comment by msabr...@gmail.com on 10 Aug 2013 at 8:25

• Changed state: Accepted
• Added labels: leak

[GoogleCodeExporter]

OK, I just applied Ramiro's patch as:

https://code.google.com/p/pymssql/source/detail?r=f549c43fe0266f637f50bd27107c36
4747ed41ac

Thanks Ramiro and Jakub!

Re: garbage collecting of mssql MSSQLConnection, I think I might know what the 
problem is and will do a commit shortly to fix it...

Original comment by msabr...@gmail.com on 13 Aug 2013 at 5:06

[GoogleCodeExporter]

And here's the other fix I mentioned, with both of these fixes, I'm not seeing 
RSS or SZ increasing over time with a script that keeps connecting and doing 
queries.

https://code.google.com/p/pymssql/source/detail?r=54554641cc7e487b9e8d5aa63b7085
bc31a5f060

Please test and let me know if it works for you...

Original comment by msabr...@gmail.com on 13 Aug 2013 at 5:15

• Changed state: Fixed

[GoogleCodeExporter]

Tested with:

- The open-select-close in a loop script from the original post for this issue:

Against:

- Code from latest pymssql default branch as of now

- Linked against FreeTDS freetds-0.91.89 (a tarball downloaded
  from ftp://ftp.astron.com/pub/freetds/stable/ -- further fixes
  to 0.91 since its release without new disruptive changes)

- With python 2.7.3 on x86_64:
  Python 2.7.3 (default, Jan  2 2013, 13:56:14)
  [GCC 4.7.2] on linux2

- From a Linux host

- Against a SQL Server 205 running on a Windows 7 host

It has executed 660K+ loops without the VIRT or RES columns values of the 
python process as reported by htop increeasing at all.

So it seems the two memory leaks are gone for good.

Original comment by cra...@gmail.com on 16 Aug 2013 at 11:51

[GoogleCodeExporter]

Thanks, Ramiro, for verifying!

Marking this verified. 

Original comment by msabr...@gmail.com on 17 Aug 2013 at 12:51

• Changed state: Verified