Issue Details (XML | Word | Printable)

Key: PYFB-49
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: Pavel Cisar
Reporter: Philipp Wojke
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Firebird driver for Python

memory and DB resource leak due to circular references

Created: 04/Mar/15 12:52 PM   Updated: 01/Apr/15 12:05 PM
Component/s: None
Affects Version/s: None
Fix Version/s: 1.4.5

File Attachments: 1. Text File fbcore.py.2.patch (3 kB)
2. Text File fbcore.py.3.patch (3 kB)
3. Text File fbcore.py.patch (2 kB)

Image Attachments:

1. objgraph.png
(59 kB)
Environment: CPython with reference count garbage collection


 Description  « Hide
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.

 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Philipp Wojke added a comment - 04/Mar/15 12:56 PM
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.

Philipp Wojke added a comment - 19/Mar/15 10:47 AM
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.

Philipp Wojke added a comment - 19/Mar/15 11:26 AM
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.

Pavel Cisar added a comment - 01/Apr/15 12:05 PM
Patch tested and accepted. Many thanks Philipp.