search

clueboy / pymssql_issues

0 stars 0 forks source link

memory leaks (patches included) #119

Closed clueboy closed 11 years ago

clueboy commented 11 years ago

From lclll.mail on April 17, 2013 00:24:07

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: http://code.google.com/p/pymssql/issues/detail?id=119

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 05:48:31

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).

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 06:00:03

Interesting, that nobody cares about memleak bug.

clueboy commented 11 years ago

From lclll.mail on June 20, 2013 06:20:31

i have to choice pyodbc ......

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 06:51:03

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)

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 06:53:54

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.

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 06:56:18

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

clueboy commented 11 years ago

From lclll.mail on June 20, 2013 07:07:26

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

clueboy commented 11 years ago

From cramm0 on June 20, 2013 07:49:49

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
clueboy commented 11 years ago

From cramm0 on June 20, 2013 07:50:48

s/serber/server/

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 07:52:30

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.

clueboy commented 11 years ago

From cramm0 on June 20, 2013 08:02:51

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!

clueboy commented 11 years ago

From jkolczasty on June 20, 2013 08:41:16

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.

clueboy commented 11 years ago

From jkolczasty on June 21, 2013 02:02:46

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

clueboy commented 11 years ago

From jkolczasty on June 21, 2013 02:14:34

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.

clueboy commented 11 years ago

From jkolczasty on June 21, 2013 02:24:48

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 ;-)

Attachment: _mssql.issue119.patch

clueboy commented 11 years ago

From cramm0 on June 21, 2013 07:14:11

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.

clueboy commented 11 years ago

From jkolczasty on June 21, 2013 08:23:43

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.

clueboy commented 11 years ago

From msabramo on August 06, 2013 14:48:05

Summary: memory leaks (patches included) (was: memory leak in windows platform)
Owner: msabramo

clueboy commented 11 years ago

From msabramo on August 06, 2013 14:55:06

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

clueboy commented 11 years ago

From msabramo on August 10, 2013 13:25:17

Status: Accepted
Labels: leak

clueboy commented 11 years ago

From msabramo on August 12, 2013 22:06:55

OK, I just applied Ramiro's patch as: https://code.google.com/p/pymssql/source/detail?r=f549c43fe0266f637f50bd27107c364747ed41ac 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...

clueboy commented 11 years ago

From msabramo on August 12, 2013 22:15:23

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=54554641cc7e487b9e8d5aa63b7085bc31a5f060 Please test and let me know if it works for you...

Status: Fixed

clueboy commented 11 years ago

From cramm0 on August 16, 2013 16:51:49

Tested with:

Against:

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.

clueboy commented 11 years ago

From msabramo on August 16, 2013 17:51:53

Thanks, Ramiro, for verifying!

Marking this verified.

Status: Verified