From 3bb62218827697ff8de06e67a191db58aec1f2aa Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 31 Jul 2026 10:09:14 +0100 Subject: [PATCH] Release aggregate instances when sqlite finishes with them The check at the top of the destroy function returns when the aggregate context still holds an instance, which is exactly when there is something to clean up. So instances were never unlinked, and every aggregate call left one behind for as long as the connection stayed open. --- ext/sqlite3/aggregator.c | 2 +- test/test_integration_aggregate.rb | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/ext/sqlite3/aggregator.c b/ext/sqlite3/aggregator.c index f2704306..9a04aa64 100644 --- a/ext/sqlite3/aggregator.c +++ b/ext/sqlite3/aggregator.c @@ -97,7 +97,7 @@ rb_sqlite3_aggregate_instance_destroy(sqlite3_context *ctx) VALUE *inst_ptr = sqlite3_aggregate_context(ctx, 0); VALUE inst; - if (!inst_ptr || (inst = *inst_ptr)) { + if (!inst_ptr || !(inst = *inst_ptr)) { return; } diff --git a/test/test_integration_aggregate.rb b/test/test_integration_aggregate.rb index aa9c3e5f..437ddd23 100644 --- a/test/test_integration_aggregate.rb +++ b/test/test_integration_aggregate.rb @@ -354,6 +354,18 @@ def finalize end end + # Used by one test only, so a live count of 1 means just the template. + class ReleasableAggregator + def step(*args) + @sum ||= 0 + args.each { |a| @sum += a.to_i } + end + + def finalize + @sum + end + end + class AccumulateAggregator2 def step(a, b) @sum ||= 1 @@ -396,6 +408,15 @@ def test_define_aggregator_does_not_use_moved_instances_after_gc_compaction assert_equal 33, values[1] end + def test_aggregate_instances_are_released_after_each_query + @db.define_aggregator("accumulate", ReleasableAggregator.new) + + 5.times { assert_equal 33, @db.get_first_value("select accumulate(c) from foo") } + GC.start(full_mark: true, immediate_sweep: true) + + assert_equal 1, ObjectSpace.each_object(ReleasableAggregator).count + end + def test_step_on_statement_whose_database_was_closed_does_not_use_freed_aggregator @db.define_aggregator("accumulate", AccumulateAggregator.new) stmt = @db.prepare("select accumulate(c) from foo")