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 and DB resource leak due to circular references [PYFB49] #67

Closed
firebird-automations opened this issue Mar 4, 2015 · 9 comments

Comments

@firebird-automations
Copy link

Submitted by: Philipp Wojke (pw)

Attachments:
fbcore.py.patch
objgraph.png
fbcore.py.2.patch
fbcore.py.3.patch

http://fbcore.py uses weakrefs for breaking reference cycles, but fails to do it properly. On several occasions a weakref callback is used, that introduces a circular reference through the implicit self of the bound method.

This leads to multiple memory leaks, the most obvious one affecting PreparedStatement and every call to execute with a different operation. This memory leak is intensified by the fact that PreparedStatement holds a database resource handle, the prepared statement. In scripts that heavily pound the database this leads to error -904 'too many open handles to database'.

To break the cycle an additional weakref can be used. Each time a callback to a method bound to self is used, this must be wrapped in a weakref proxy:

Wrong: self.cursor = weakref.proxy(cursor,self.__cursor_deleted)
Right: self.cursor = weakref.proxy(cursor,weakref.proxy(self.__cursor_deleted))

No additional handling should be necessary, because the referenced method lives at least as long as the weakref calling the callback.

This change fixed multiple previously unexplained program terminations and memory problems in our environment.

Commits: 148f23f FirebirdSQL/fbt-repository@2f06fd5

@firebird-automations
Copy link
Author

Commented by: Philipp Wojke (pw)

Suggested patch for svn revision 60825.
If there are newer revisions before this patch is applied all usages of weakref callbacks should be checked for described bug.

@firebird-automations
Copy link
Author

Modified by: Philipp Wojke (pw)

Attachment: fbcore.py.patch [ 12680 ]

@firebird-automations
Copy link
Author

Modified by: Philipp Wojke (pw)

Attachment: objgraph.png [ 12681 ]

@firebird-automations
Copy link
Author

Commented by: Philipp Wojke (pw)

Additional change to Transaction.__close_cursors to ignore already deleted cursors. Otherwise an exceptions may be raised when closing cursors from Transaction.__del__ if the cursor was already garbage collected.

There may be some more places where weakly referenced objects that had been already garbage collected are accessed from __del__ methods.

@firebird-automations
Copy link
Author

Modified by: Philipp Wojke (pw)

Attachment: fbcore.py.2.patch [ 12686 ]

@firebird-automations
Copy link
Author

Commented by: Philipp Wojke (pw)

Forget to include something in patch 2. Use a special proxy instead of weakref.proxy to only call callback used in weakref if callback function still exists.

@firebird-automations
Copy link
Author

Modified by: Philipp Wojke (pw)

Attachment: fbcore.py.3.patch [ 12687 ]

@firebird-automations
Copy link
Author

Modified by: @pcisar

status: Open [ 1 ] => Closed [ 6 ]

resolution: Fixed [ 1 ]

Fix Version: 1.4.5 [ 10671 ]

@firebird-automations
Copy link
Author

Commented by: @pcisar

Patch tested and accepted. Many thanks Philipp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants