Skip to content

Commit

Permalink
Make sure we close all database file descriptors.
Browse files Browse the repository at this point in the history
Iterate through the names returned by sqlite3_db_name which will
include names given via "ATTACH DATABASE ... AS" and any temp
databases.
  • Loading branch information
flavorjones committed Sep 17, 2024
1 parent 2bb2341 commit 05ede01
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 12 deletions.
13 changes: 8 additions & 5 deletions ext/sqlite3/database.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,20 @@ close_or_discard_db(sqlite3RubyPtr ctx)
} else {
// This is an open connection carried across a fork().
// "Discard" it. See adr/2024-09-fork-safety.md
const char *db_name;
sqlite3_file *sfile;
int status;
int status, j_db = 0;

rb_warning("An open sqlite database connection was inherited from a forked process and "
"is being discarded. This is a memory leak. If possible, please close all sqlite "
"database connections before forking.");

// close the open file descriptors
status = sqlite3_file_control(ctx->db, NULL, SQLITE_FCNTL_FILE_POINTER, &sfile);
if (status == 0 && sfile->pMethods != NULL) {
sfile->pMethods->xClose(sfile);
while ((db_name = sqlite3_db_name(ctx->db, j_db)) != NULL) {
status = sqlite3_file_control(ctx->db, db_name, SQLITE_FCNTL_FILE_POINTER, &sfile);
if (status == 0 && sfile->pMethods != NULL) {
sfile->pMethods->xClose(sfile);
}
j_db++;
}

status = sqlite3_file_control(ctx->db, NULL, SQLITE_FCNTL_JOURNAL_POINTER, &sfile);
Expand Down
18 changes: 11 additions & 7 deletions test/test_database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,9 @@ def test_discard_a_connection
read, write = IO.pipe

db = SQLite3::Database.new("test.db")
db.execute("attach database 'test.db' as 'foo';") # exercise sqlite3_db_name()
Process.fork do
read.close
$stderr = StringIO.new

result = db.close
Expand All @@ -740,20 +742,22 @@ def test_discard_a_connection
write.write($stderr.string)

write.close
read.close
exit!
end
write.close

assert1, assert2, *stderr = *read.readlines
read.close

assert_equal("ok", read.readline.chomp, "return value was not the database")
assert_equal("ok", read.readline.chomp, "closed? did not return true")
assert_equal("ok", assert1.chomp, "return value was not the database")
assert_equal("ok", assert2.chomp, "closed? did not return true")

assert_equal(1, stderr.count, "unexpected output on stderr: #{stderr.inspect}")
assert_match(
/warning: An open sqlite database connection was inherited from a forked process/,
read.readline,
stderr.first,
"expected warning was not emitted"
)

write.close
read.close
ensure
db.close
FileUtils.rm_f("test.db")
Expand Down

0 comments on commit 05ede01

Please sign in to comment.