Add explicit deterministic close support for SQLite connections#1361
Add explicit deterministic close support for SQLite connections#1361RuiNelson wants to merge 4 commits into
Conversation
Add an explicit Connection.close() API that synchronously closes the underlying sqlite3 handle and only clears it after sqlite3_close returns SQLITE_OK. Previously Connection.deinit called sqlite3_close directly and ignored its result. If SQLite still had internal statements, BLOB handles, or backup objects alive, sqlite3_close could return SQLITE_BUSY and leave the database connection open. That made the Swift object appear destroyed while SQLite still held file descriptors, which could trigger iOS filesystem warnings when temporary database files were unlinked shortly afterwards. The deinitializer now delegates to close(), preserving the existing automatic cleanup path while avoiding silent handle leaks when explicit close succeeds.
Add regression coverage for the explicit Connection.close() API. The new tests verify that close() is idempotent, reports SQLITE_BUSY while a prepared statement is still active, leaves the connection usable after a failed close attempt, and succeeds once the statement has been released.
|
Thanks for the PR! Can you document this change in Documentation/Index.md and rebase the PR against the latest master so that the tests get a chance to run? |
Co-authored-by: Cursor <cursoragent@cursor.com>
Done. I tried to maintain the level of verbosity and tone of the rest of the document. |
|
|
||
| let resultCode = sqlite3_close(handle) | ||
| if resultCode == SQLITE_OK { | ||
| _handle = nil |
There was a problem hiding this comment.
This will put the Connection object in a state where most subsequent operations will fail while force unwrapping the handle (line 88).
The intention of having the sqlit3_close happen in the deinit was probably to avoid this: either the object is constructed, and usable, or it's no longer in scope (and closed). Being able to close this explicitly puts the connection in a gray zone.
Perhaps the code unwrapping the handle could make this explicit:
public var handle: OpaquePointer {
assert(_handle != nil, "connection closed")
return _handle!
}The code will still fail, but the error message will be clearer.
Sounds like something an AI would say when prompted to write documentation :) |
Actually, it was what I told my AI agent to translate my Portuguese draft to English :o) Or maybe I'm a machine inside an human-like body that was programmed to think it is human. We'll never know! |
Haha, ok! What do you think about the other suggestions? A more explicit failure when the connection is closed? |
|
I was thinking about throwing an error like, but I need to check the codebase first... func getHandle() throws(ConnectionAlreadyClosed) {
guard let handle else {
throw ConnectionAlreadyClosed()
}
return handle
}
// and then, on each method:
let handle = try getHandle() |
2 participants
This PR adds an explicit
Connection.close()API so callers can deterministically close a SQLite database connection before releasing or deleting the underlying database file.Motivation
Some applications need to manage short-lived SQLite database files explicitly. In those cases, relying only on
Connection.deinitcan make file lifetime difficult to coordinate with external cleanup.This is especially important when a connection may still have active SQLite resources, such as prepared statements. SQLite’s
sqlite3_close()returnsSQLITE_BUSYin that case and keeps the connection open. Without an explicit close API, callers cannot observe that failure or retry the close after releasing the remaining resources.Solution
The new
Connection.close()method:sqlite3_close()synchronously on the connection queue.nilonly when SQLite reportsSQLITE_OK.deinit, preserving the previous automatic cleanup behavior.Tests
Added regression coverage for the new behavior:
close()succeeds and is idempotent.close()throwsSQLITE_BUSYwhile a prepared statement is still active.close()succeeds after the active statement is released.