Always call sqlite3_finalize in deallocate func - #392
Conversation
|
@haileys Thanks for submitting this PR. Unfortunately, this is insufficient to prevent memory leaks -- I've added some tests that fail when I think this is happening because we can't guarantee that the statement will be finalized before the database is closed. See Closing A Database Connection for more information on what happens when the order is reversed. |
9574673 to
e57e04b
Compare
|
I've rebased this onto current origin/main. |
e57e04b to
3ba5cd0
Compare
I suspect you're not wrong. But I think we might (possibly maybe) be able to do a GC trick that keeps the connection open 1 GC cycle past the statement. AFAIU, the only time we wouldn't be able to guarantee this order is in the case a database object and a statement object are collected at the same time. I think there's a way we can force the connection to stay alive for one more GC. IIRC we do this in CRuby for classes, but idk if the C APIs are public. |
prevents memory leak when `close` is not called before a ResultSet is garbage collected also add coverage for statement resource cleanup Co-authored-by: Mike Dalessio <mike.dalessio@gmail.com>
3ba5cd0 to
0c24631
Compare
tenderlove
left a comment
There was a problem hiding this comment.
We might need more tricks in case a db and statement are collected simultaneously, but this is a good start.
Prevents memory leak in the case that
closeis not called before aResultSetis garbage collected.closesetsc->sttoNULLimmediately after callingsqlite3_finalizeitself, so there is no double-free risk introduced with this change.