search

djhenderson / pyodbc

Automatically exported from code.google.com/p/pyodbc
MIT No Attribution
0 stars 0 forks source link

cursor.execute leaks parameters #145

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
The parameters to cursor.execute are being leaked.  This is really noticeable 
when INSERTing a lot of data.  The problem is due to PySequence_GetItem (which 
returns a *new* reference) being used on the parameter list to pick out each 
parameter, but this reference is never released.

A test case patch and a fix patch are attached.

Original issue reported on code.google.com by lukedell...@gmail.com on 17 Dec 2010 at 1:53

Attachments:

GoogleCodeExporter commented 9 years ago 
I actually INSERT a lot of data, and found out this leak as well.
I cannot run the patch (not used to) and need to solve this issue.
Would you have another way to solve this?

Original comment by cy...@ikos.com.cy on 22 Dec 2010 at 8:38

GoogleCodeExporter commented 9 years ago 
I have applied this patch to current github trunk (7580498b9f8c8c7e08ad). It 
applies cleanly and fixes the memory leak.

Original comment by lamba...@gmail.com on 13 May 2011 at 3:53

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
Attempting to apply this patch to either the trunk or v2.1.8 on OS X results in 
errors:

patch < pyodbc-paramleak-fix.patch 

patching file cursor.h
patching file params.cpp
Hunk #1 FAILED at 20.
Hunk #2 FAILED at 457.
Hunk #3 FAILED at 567.
Hunk #4 FAILED at 647.
4 out of 4 hunks FAILED -- saving rejects to file params.cpp.rej

Original comment by bsg...@gmail.com on 18 Jun 2011 at 3:23

GoogleCodeExporter commented 9 years ago 
This looks like a text file line ending issue.  I see that in git trunk & 
pyodbc-2.1.8.zip there is a mixture of Windows-style CR+LF line endings, and 
Unix-style LF line endings.  In this specific case cursor.h uses LF and so 
works fine for you, but params.cpp uses CR+LF and so you get these conflicts.  
Not sure if I should adjust the patch to take this into account?  Meanwhile if 
you convert params.cpp to Unix-style LF line endings then the existing patch 
should work fine.

Original comment by lukedell...@gmail.com on 19 Jun 2011 at 3:04

GoogleCodeExporter commented 9 years ago 
Normalizing line endings allows patch application, and the patch appears to 
resolve the memory leak.

Issue 184 entered for the mixed line endings.

Original comment by bsg...@gmail.com on 20 Jun 2011 at 4:18

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
Verified that this patch resolves the leak, tested on Win64 and OS X 10.6. An 
ETL script with 38 hour average runs was consuming up to 30GB of RAM using 
parameterized inserts on unpatched 2.1.8

Applying the patch keeps memory consumption under 20MB for the same script.

Original comment by bsg...@gmail.com on 22 Jun 2011 at 3:42

GoogleCodeExporter commented 9 years ago 
I sincerely appreciate the fix!  I apologize for missed this earlier.

I've used the fix you suggested and am releasing 2.1.10 for this.

Original comment by mkleehammer on 13 Sep 2011 at 1:28

GoogleCodeExporter commented 9 years ago 
Will you be adding a 64-bit Windows binary?

Original comment by bsg...@gmail.com on 13 Sep 2011 at 8:52

GoogleCodeExporter commented 9 years ago 
I was able to build on 64 bit Win2008, but I had to add the utils path to the 
source from git. It was not part of the source zip.

Original comment by bsg...@gmail.com on 13 Sep 2011 at 9:18