Skip to content

Commit 20025b4

Browse files
flavorjonesflavorjones
committed
Statements attached to a discarded db raise an exception
1 parent f34e812 commit 20025b4

File tree

3 files changed

+121
-33
lines changed

3 files changed

+121
-33
lines changed

‎ext/sqlite3/database.c‎

Lines changed: 57 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,45 @@
1212

1313
VALUE cSqlite3Database;
1414

15+
/* See adr/2024-09-fork-safety.md */
16+
static void
17+
discard_db(sqlite3RubyPtr ctx)
18+
{
19+
sqlite3_file *sfile;
20+
int status;
21+
22+
// release as much heap memory as possible by deallocating non-essential memory
23+
// allocations held by the database library. Memory used to cache database pages to
24+
// improve performance is an example of non-essential memory.
25+
// on my development machine, this reduces the lost memory from 152k to 69k.
26+
sqlite3_db_release_memory(ctx->db);
27+
28+
// release file descriptors
29+
#ifdef HAVE_SQLITE3_DB_NAME
30+
const char *db_name;
31+
int j_db = 0;
32+
while ((db_name = sqlite3_db_name(ctx->db, j_db)) != NULL) {
33+
status = sqlite3_file_control(ctx->db, db_name, SQLITE_FCNTL_FILE_POINTER, &sfile);
34+
if (status == 0 && sfile->pMethods != NULL) {
35+
sfile->pMethods->xClose(sfile);
36+
}
37+
j_db++;
38+
}
39+
#else
40+
status = sqlite3_file_control(ctx->db, NULL, SQLITE_FCNTL_FILE_POINTER, &sfile);
41+
if (status == 0 && sfile->pMethods != NULL) {
42+
sfile->pMethods->xClose(sfile);
43+
}
44+
#endif
45+
46+
status = sqlite3_file_control(ctx->db, NULL, SQLITE_FCNTL_JOURNAL_POINTER, &sfile);
47+
if (status == 0 && sfile->pMethods != NULL) {
48+
sfile->pMethods->xClose(sfile);
49+
}
50+
51+
ctx->db = NULL;
52+
}
53+
1554
static void
1655
close_or_discard_db(sqlite3RubyPtr ctx)
1756
{
@@ -21,41 +60,11 @@ close_or_discard_db(sqlite3RubyPtr ctx)
2160
if (isReadonly || ctx->owner == getpid()) {
2261
// Ordinary close.
2362
sqlite3_close_v2(ctx->db);
63+
ctx->db = NULL;
2464
} else {
25-
// This is an open connection carried across a fork().
26-
// "Discard" it. See adr/2024-09-fork-safety.md
27-
sqlite3_file *sfile;
28-
int status;
29-
30-
// release as much heap memory as possible by deallocating non-essential memory
31-
// allocations held by the database library. Memory used to cache database pages to
32-
// improve performance is an example of non-essential memory.
33-
sqlite3_db_release_memory(ctx->db);
34-
35-
// release file descriptors
36-
#ifdef HAVE_SQLITE3_DB_NAME
37-
const char *db_name;
38-
int j_db = 0;
39-
while ((db_name = sqlite3_db_name(ctx->db, j_db)) != NULL) {
40-
status = sqlite3_file_control(ctx->db, db_name, SQLITE_FCNTL_FILE_POINTER, &sfile);
41-
if (status == 0 && sfile->pMethods != NULL) {
42-
sfile->pMethods->xClose(sfile);
43-
}
44-
j_db++;
45-
}
46-
#else
47-
status = sqlite3_file_control(ctx->db, NULL, SQLITE_FCNTL_FILE_POINTER, &sfile);
48-
if (status == 0 && sfile->pMethods != NULL) {
49-
sfile->pMethods->xClose(sfile);
50-
}
51-
#endif
52-
53-
status = sqlite3_file_control(ctx->db, NULL, SQLITE_FCNTL_JOURNAL_POINTER, &sfile);
54-
if (status == 0 && sfile->pMethods != NULL) {
55-
sfile->pMethods->xClose(sfile);
56-
}
65+
// This is an open connection carried across a fork(). "Discard" it.
66+
discard_db(ctx);
5767
}
58-
ctx->db = NULL;
5968
}
6069
}
6170

@@ -190,6 +199,20 @@ sqlite3_rb_close(VALUE self)
190199
return self;
191200
}
192201

202+
/* private method, primarily for testing */
203+
static VALUE
204+
sqlite3_rb_discard(VALUE self)
205+
{
206+
sqlite3RubyPtr ctx;
207+
TypedData_Get_Struct(self, sqlite3Ruby, &database_type, ctx);
208+
209+
discard_db(ctx);
210+
211+
rb_iv_set(self, "-aggregators", Qnil);
212+
213+
return self;
214+
}
215+
193216
/* call-seq: db.closed?
194217
*
195218
* Returns +true+ if this database instance has been closed (see #close).
@@ -943,6 +966,7 @@ init_sqlite3_database(void)
943966
rb_define_private_method(cSqlite3Database, "open16", rb_sqlite3_open16, 1);
944967
rb_define_method(cSqlite3Database, "collation", collation, 2);
945968
rb_define_method(cSqlite3Database, "close", sqlite3_rb_close, 0);
969+
rb_define_private_method(cSqlite3Database, "discard", sqlite3_rb_discard, 0);
946970
rb_define_method(cSqlite3Database, "closed?", closed_p, 0);
947971
rb_define_method(cSqlite3Database, "total_changes", total_changes, 0);
948972
rb_define_method(cSqlite3Database, "trace", trace, -1);

‎ext/sqlite3/statement.c‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@
44
if(!_ctxt->st) \
55
rb_raise(rb_path2class("SQLite3::Exception"), "cannot use a closed statement");
66

7+
static void
8+
require_open_db(VALUE stmt_rb)
9+
{
10+
VALUE closed_p = rb_funcall(
11+
rb_iv_get(stmt_rb, "@connection"),
12+
rb_intern("closed?"), 0);
13+
14+
if (RTEST(closed_p)) {
15+
rb_raise(rb_path2class("SQLite3::Exception"),
16+
"cannot use a statement associated with a closed database");
17+
}
18+
}
19+
20+
721
VALUE cSqlite3Statement;
822

923
static void
@@ -121,6 +135,7 @@ step(VALUE self)
121135

122136
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
123137

138+
require_open_db(self);
124139
REQUIRE_OPEN_STMT(ctx);
125140

126141
if (ctx->done_p) { return Qnil; }
@@ -216,6 +231,8 @@ bind_param(VALUE self, VALUE key, VALUE value)
216231
int index;
217232

218233
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
234+
235+
require_open_db(self);
219236
REQUIRE_OPEN_STMT(ctx);
220237

221238
switch (TYPE(key)) {
@@ -308,6 +325,8 @@ reset_bang(VALUE self)
308325
sqlite3StmtRubyPtr ctx;
309326

310327
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
328+
329+
require_open_db(self);
311330
REQUIRE_OPEN_STMT(ctx);
312331

313332
sqlite3_reset(ctx->st);
@@ -328,6 +347,8 @@ clear_bindings_bang(VALUE self)
328347
sqlite3StmtRubyPtr ctx;
329348

330349
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
350+
351+
require_open_db(self);
331352
REQUIRE_OPEN_STMT(ctx);
332353

333354
sqlite3_clear_bindings(ctx->st);
@@ -360,6 +381,8 @@ column_count(VALUE self)
360381
{
361382
sqlite3StmtRubyPtr ctx;
362383
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
384+
385+
require_open_db(self);
363386
REQUIRE_OPEN_STMT(ctx);
364387

365388
return INT2NUM(sqlite3_column_count(ctx->st));
@@ -391,6 +414,8 @@ column_name(VALUE self, VALUE index)
391414
const char *name;
392415

393416
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
417+
418+
require_open_db(self);
394419
REQUIRE_OPEN_STMT(ctx);
395420

396421
name = sqlite3_column_name(ctx->st, (int)NUM2INT(index));
@@ -414,6 +439,8 @@ column_decltype(VALUE self, VALUE index)
414439
const char *name;
415440

416441
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
442+
443+
require_open_db(self);
417444
REQUIRE_OPEN_STMT(ctx);
418445

419446
name = sqlite3_column_decltype(ctx->st, (int)NUM2INT(index));
@@ -431,6 +458,8 @@ bind_parameter_count(VALUE self)
431458
{
432459
sqlite3StmtRubyPtr ctx;
433460
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
461+
462+
require_open_db(self);
434463
REQUIRE_OPEN_STMT(ctx);
435464

436465
return INT2NUM(sqlite3_bind_parameter_count(ctx->st));
@@ -538,7 +567,10 @@ stats_as_hash(VALUE self)
538567
{
539568
sqlite3StmtRubyPtr ctx;
540569
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
570+
571+
require_open_db(self);
541572
REQUIRE_OPEN_STMT(ctx);
573+
542574
VALUE arg = rb_hash_new();
543575

544576
stmt_stat_internal(arg, ctx->st);
@@ -554,6 +586,8 @@ stat_for(VALUE self, VALUE key)
554586
{
555587
sqlite3StmtRubyPtr ctx;
556588
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
589+
590+
require_open_db(self);
557591
REQUIRE_OPEN_STMT(ctx);
558592

559593
if (SYMBOL_P(key)) {
@@ -574,6 +608,8 @@ memused(VALUE self)
574608
{
575609
sqlite3StmtRubyPtr ctx;
576610
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
611+
612+
require_open_db(self);
577613
REQUIRE_OPEN_STMT(ctx);
578614

579615
return INT2NUM(sqlite3_stmt_status(ctx->st, SQLITE_STMTSTATUS_MEMUSED, 0));
@@ -591,6 +627,8 @@ database_name(VALUE self, VALUE index)
591627
{
592628
sqlite3StmtRubyPtr ctx;
593629
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
630+
631+
require_open_db(self);
594632
REQUIRE_OPEN_STMT(ctx);
595633

596634
return SQLITE3_UTF8_STR_NEW2(
@@ -608,6 +646,8 @@ get_sql(VALUE self)
608646
{
609647
sqlite3StmtRubyPtr ctx;
610648
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
649+
650+
require_open_db(self);
611651
REQUIRE_OPEN_STMT(ctx);
612652

613653
return rb_obj_freeze(SQLITE3_UTF8_STR_NEW2(sqlite3_sql(ctx->st)));
@@ -626,6 +666,8 @@ get_expanded_sql(VALUE self)
626666
VALUE rb_expanded_sql;
627667

628668
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
669+
670+
require_open_db(self);
629671
REQUIRE_OPEN_STMT(ctx);
630672

631673
expanded_sql = sqlite3_expanded_sql(ctx->st);

‎test/test_database.rb‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,5 +869,27 @@ def test_close_does_not_discard_readonly_connections
869869
FileUtils.rm_f("test.db")
870870
end
871871
end
872+
873+
def test_a_discarded_connection_with_statements
874+
skip("discard leaks memory") if i_am_running_in_valgrind
875+
876+
begin
877+
db = SQLite3::Database.new("test.db")
878+
db.execute("create table foo (bar int)")
879+
db.execute("insert into foo values (1)")
880+
stmt = db.prepare("select * from foo")
881+
882+
db.send(:discard)
883+
884+
e = assert_raises(SQLite3::Exception) { stmt.execute }
885+
assert_match(/cannot use a statement associated with a closed database/, e.message)
886+
887+
assert_nothing_raised { stmt.close }
888+
assert_predicate(stmt, :closed?)
889+
ensure
890+
db.close
891+
FileUtils.rm_f("test.db")
892+
end
893+
end
872894
end
873895
end

0 commit comments

Comments
 (0)