Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak after getting [42000][ODBC Driver 17 for SQL Server] (1105) (SQLParamData) Error #702

Closed
Mizaro opened this issue Feb 8, 2020 · 10 comments · Fixed by #703
Closed
Milestone

Comments

@Mizaro
Copy link
Contributor

Mizaro commented Feb 8, 2020

Environment

  • Python: Python 2.7 64-bit, Python 3.6 32-bit, Python 3.6 64-bit, Python 3.7 64-bit
  • pyodbc: 4.0.29b4+master
  • OS: Windows 10 64-bit
  • DB: SQL Server 2017
  • driver:

Issue

Given the following code:

KB = 1024
MB = KB * 1024

conn = pyodbc.connect('DRIVER={ODBC Driver 17 for SQL Server};SERVER=localhost;DATABASE=Full;UID=user;PWD=password')
cursor = conn.cursor()

cur = cursor.execute("INSERT INTO FullTable VALUES (?)", "a" * 10 * MB)

I got the following error:

pyodbc.ProgrammingError: ('42000', "[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Could not allocate space for object 'dbo.FullTable'.'PK_FullTable' in database 'Full' because the 'PRIMARY' filegroup is full. Create disk space by deleting unneeded files, dropping objects in the filegroup, adding additional files to the filegroup, or setting autogrowth on for existing files in the filegroup. (1105) (SQLParamData)")

The fact that the error is happending is fine. The problem is with the memory usage of the python.exe process. The expected behavior will be a small amount of new memory in use due to python's default behavior but the observed behavior is that there is about 20MB of additional memory usage. Even after calling GC or trying a lot of other methods, the memory still in use.

I have debugged this Issue and created a fork with a branch called bugfix/SQLParamData_MemoryLeak that fixes this problem.

@mkleehammer
Copy link
Owner

Well, I can see there would be an increase in memory, but I don't think it is where you are looking. When inserting a string parameter, ParameterValuePtr is pointing at the string's internal buffer. It isn't a new copy. Therefore if you free it, you are freeing the string's internal memory which is going to lead to a double-free and a crash or at least security issue.

If I'm right about this, the problem is that the parameter did not get garbage collected. I don't know what the code looked like that you originally noticed the issue in, but make sure two things are true:

  1. The parameter is out of scope (so just leave the function, for example).
  2. The cursor is closed. Manually close it, set it to None, or leave the function.

In the test code I saw, the parameter would be out of scope, but the cursor was not closed, so it still had a reference to the parameter. I think the memory would be freed as soon as you (1) closed the cursor or (2) executed a different statement.

Do you mind testing that? In fact, it wouldn't be hard to test using your existing test. Just remove the C++ changes and update your test to close the cursor and connection before measuring memory a second time.

@Mizaro
Copy link
Contributor Author

Mizaro commented Feb 8, 2020

Well, I can see there would be an increase in memory, but I don't think it is where you are looking. When inserting a string parameter, ParameterValuePtr is pointing at the string's internal buffer. It isn't a new copy. Therefore if you free it, you are freeing the string's internal memory which is going to lead to a double-free and a crash or at least security issue.

Initialiy the code I wanted to add was only Py_XDECREF(pInfo->pObject); because this is the object that holds the memory. I have addressed ParameterValuePtr too because I was there is another location in the code where Py_XDECREF(pInfo->pObject); is used, under else if (pInfo->ParameterType == SQL_SS_TABLE) inside of the for loop. If I remove the address to ParameterValuePtr, my fix still works.

If I'm right about this, the problem is that the parameter did not get garbage collected. I don't know what the code looked like that you originally noticed the issue in, but make sure two things are true:

  1. The parameter is out of scope (so just leave the function, for example).
  2. The cursor is closed. Manually close it, set it to None, or leave the function.

In the test code I saw, the parameter would be out of scope, but the cursor was not closed, so it still had a reference to the parameter. I think the memory would be freed as soon as you (1) closed the cursor or (2) executed a different statement.

I will edit my test to close and Nullify the object, then froce the GC.

Do you mind testing that? In fact, it wouldn't be hard to test using your existing test. Just remove the C++ changes and update your test to close the cursor and connection before measuring memory a second time.

Happily, I have tested it and post a photo for the results.

2020-02-08_191304_MemoryLeak_pyodbc

@Mizaro
Copy link
Contributor Author

Mizaro commented Feb 8, 2020

I continue to debug the code wondering where could the double-free happen and encountered with the following situation:

2020-02-08_191304_MemoryLeak_pyodbc_2

Seems like count's value is 0 while it should be 1. Right now trying to see where the increase of paramcount should have happened.

@Mizaro
Copy link
Contributor Author

Mizaro commented Feb 8, 2020

After some more debugging I have changed my fix. Please take another look.

I have swapped the order of

FreeParameterData(cur);
FreeParameterInfo(cur);

in closeimpl

but I still think maybe FreeParameterData should be called before raising error on SQLParamData.

@gordthompson
Copy link
Collaborator

Possibly related: #802

@jfuernsinn
Copy link

I am having the same issue described in this ticket or #802, is there any timeline associated with having the #703 fix merged to master?

@gordthompson
Copy link
Collaborator

gordthompson commented Nov 22, 2020

@jfuernsinn ; cc: @Mizaro @mkleehammer

I just tested the #703 patch and unfortunately it does not appear to resolve the #802 issue:

using varchar(max)
pyodbc 4.0.30
startup: vms 6.3 MiB
data loaded: vms 16.8 MiB
iteration 0: vms 22.8 MiB, 54.2 sec.
iteration 1: vms 26.8 MiB, 51.8 sec.
iteration 2: vms 30.5 MiB, 52.4 sec.
iteration 3: vms 34.5 MiB, 51.9 sec.
iteration 4: vms 38.2 MiB, 52.5 sec.

----------

using varchar(max)
pyodbc 4.0.31b11+commit6d4e0a8
startup: vms 6.3 MiB
data loaded: vms 16.8 MiB
iteration 0: vms 22.8 MiB, 52.4 sec.
iteration 1: vms 26.9 MiB, 57.0 sec.
iteration 2: vms 30.5 MiB, 58.6 sec.
iteration 3: vms 34.5 MiB, 54.1 sec.
iteration 4: vms 38.2 MiB, 59.8 sec.

@gordthompson
Copy link
Collaborator

And here's a slightly longer test, just to see if garbage collection might eventually kick in:

using varchar(max)
pyodbc 4.0.31b11+commit6d4e0a8
startup: vms 6.3 MiB
data loaded: vms 16.8 MiB
iteration 0: vms 22.8 MiB, 52.3 sec.
iteration 1: vms 26.8 MiB, 52.2 sec.
iteration 2: vms 30.5 MiB, 53.0 sec.
iteration 3: vms 34.5 MiB, 60.4 sec.
iteration 4: vms 38.2 MiB, 57.7 sec.
iteration 5: vms 42.2 MiB, 56.6 sec.
iteration 6: vms 46.1 MiB, 58.9 sec.
iteration 7: vms 50.0 MiB, 56.7 sec.
iteration 8: vms 53.8 MiB, 58.0 sec.
iteration 9: vms 57.8 MiB, 59.3 sec.
iteration 10: vms 61.5 MiB, 59.9 sec.
iteration 11: vms 65.5 MiB, 58.4 sec.
iteration 12: vms 69.3 MiB, 60.8 sec.
iteration 13: vms 73.3 MiB, 59.3 sec.
iteration 14: vms 77.1 MiB, 57.8 sec.
iteration 15: vms 81.1 MiB, 56.3 sec.
iteration 16: vms 84.8 MiB, 58.2 sec.
iteration 17: vms 88.6 MiB, 59.3 sec.
iteration 18: vms 92.6 MiB, 58.6 sec.
iteration 19: vms 96.3 MiB, 53.3 sec.
iteration 20: vms 100.3 MiB, 59.9 sec.
iteration 21: vms 104.1 MiB, 58.5 sec.
iteration 22: vms 108.1 MiB, 59.0 sec.
iteration 23: vms 111.9 MiB, 59.8 sec.
iteration 24: vms 115.9 MiB, 60.9 sec.
iteration 25: vms 119.6 MiB, 58.4 sec.
iteration 26: vms 123.6 MiB, 63.1 sec.
iteration 27: vms 127.4 MiB, 59.3 sec.
iteration 28: vms 131.4 MiB, 59.4 sec.
iteration 29: vms 135.2 MiB, 61.4 sec.

@Mizaro
Copy link
Contributor Author

Mizaro commented Nov 23, 2020

@gordthompson
Take a look at #832, I think I have solved this issue there.

@gordthompson
Copy link
Collaborator

@Mizaro - Yes, indeed, that patch does seem to fix it:

using varchar(max)
pyodbc 4.0.31b5+commit1ca0ee0
startup: vms 6.3 MiB
data loaded: vms 16.8 MiB
iteration 0: vms 19.1 MiB, 53.8 sec.
iteration 1: vms 19.1 MiB, 56.5 sec.
iteration 2: vms 18.9 MiB, 53.7 sec.
iteration 3: vms 18.9 MiB, 53.8 sec.
iteration 4: vms 18.9 MiB, 53.1 sec.

@gordthompson gordthompson reopened this May 29, 2021
@gordthompson gordthompson linked a pull request Jun 10, 2021 that will close this issue
@gordthompson gordthompson added this to the 4.0.37 milestone Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants