Skip to content

Commit bd194dc

Browse files
committed
unwind owned rows on pool release failure
1 parent af029ca commit bd194dc

4 files changed

Lines changed: 140 additions & 4 deletions

File tree

‎docs/FEATURE_MATRIX.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ coverage; PostgreSQL rows marked live also run against PostgreSQL 16 in CI.
2020
| Safe parameter binding | Complete | positional/named APIs; single-allocation PostgreSQL Bind packets; full unsigned 16-bit parameter count; strict protocol C-strings | byte-parity/allocation/NUL tests; driver tests; `tests/postgres_live.zig` |
2121
| Typed row decoding | Complete | `Row.as`, `Row.asName`, `Row.to`, `zsql.decode`; owned PostgreSQL bytea hex/escape decode; single-schema query contract | malformed/OOM unit tests; multi-result recovery and core/live driver tests |
2222
| Explicit owned rows | Complete | `OwnedRow`, `Row.getOwned`, `zsql.freeOwnedRows`; documented invalidation boundary for borrowed values | allocator-backed core/driver tests plus external SQLite survival after rows, connection, and database teardown |
23-
| Connection pooling | Complete | `Pool(D)`, `Lease(D)`, health-aware consuming release, OOM-safe idle return, shutdown draining/wakeup, stats and timeouts | SQLite recovery/lifecycle tests; PostgreSQL live release-OOM/shutdown/recovery tests |
23+
| Connection pooling | Complete | `Pool(D)`, `Lease(D)`, health-aware consuming release, OOM-safe idle return and owned-result unwind, shutdown draining/wakeup, stats and timeouts | SQLite release-OOM/result-unwind/recovery tests; PostgreSQL live release-OOM/result-unwind/shutdown tests |
2424
| Transactions and savepoints | Complete | explicit nested/idle/aborted states; PostgreSQL `25P02` mapping; prepared-statement transition safety; failed-state savepoint recovery; `Tx(D)`, `Savepoint(D)`, `withTx` | SQLite state tests and PostgreSQL direct/prepared/pool live tests |
2525
| Migrations | Complete | transactional apply, durable dirty failures, checksum-guarded API/CLI repair; `Migrator(D).up`, `.status` | SQLite repair workflow, PostgreSQL live repair tests, CLI parser tests, migration example |
2626
| Schema inspection | Complete | driver `inspectSchema`; dialect-tagged CLI `inspect`; structured PostgreSQL schema/table identity; self-contained nullable struct generation with exact fields and schema-aware collision-free table types | inspector/codegen syntax tests and PostgreSQL live tests |

‎src/drivers/postgres/pool.zig‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,11 @@ pub const Pool = struct {
260260
pub fn queryOneParams(self: *Pool, sql: []const u8, binds: []const core.Value) !core.OwnedRow {
261261
var lease = try self.acquire();
262262
errdefer if (lease.open) lease.discard() catch {};
263-
const owned = (try lease.conn()).queryOneParams(sql, binds) catch |err| {
263+
var owned = (try lease.conn()).queryOneParams(sql, binds) catch |err| {
264264
lease.finishAfterError(err);
265265
return err;
266266
};
267+
errdefer owned.deinit();
267268
try lease.release();
268269
return owned;
269270
}
@@ -277,6 +278,7 @@ pub const Pool = struct {
277278
lease.finishAfterError(err);
278279
return err;
279280
};
281+
errdefer core.OwnedRow.freeSlice(self.allocator, owned);
280282
try lease.release();
281283
return owned;
282284
}

‎src/drivers/sqlite/sqlite.zig‎

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,10 +323,11 @@ pub const Pool = struct {
323323
pub fn queryOne(self: *Pool, sql: []const u8, binds: []const core.Value) !core.OwnedRow {
324324
var lease = try self.acquire();
325325
errdefer if (lease.open) lease.discard() catch {};
326-
const owned = (try lease.conn()).queryOne(sql, binds) catch |err| {
326+
var owned = (try lease.conn()).queryOne(sql, binds) catch |err| {
327327
lease.finishAfterError(err);
328328
return err;
329329
};
330+
errdefer owned.deinit();
330331
try lease.release();
331332
return owned;
332333
}
@@ -340,6 +341,7 @@ pub const Pool = struct {
340341
lease.finishAfterError(err);
341342
return err;
342343
};
344+
errdefer core.OwnedRow.freeSlice(self.allocator, owned);
343345
try lease.release();
344346
return owned;
345347
}
@@ -431,7 +433,14 @@ pub const Lease = struct {
431433
}
432434

433435
if (self.pool.idle.items.len < self.pool.effectiveMaxIdle()) {
434-
try self.pool.idle.append(self.pool.allocator, self.db);
436+
self.pool.idle.append(self.pool.allocator, self.db) catch |err| {
437+
self.conn_value.close();
438+
self.db.deinit();
439+
self.pool.open_count -|= 1;
440+
self.open = false;
441+
self.pool.notifyAvailable();
442+
return err;
443+
};
435444
self.conn_value.close();
436445
} else {
437446
self.conn_value.close();
@@ -3000,6 +3009,72 @@ test "SQLite pool timed acquire unblocks after concurrent release" {
30003009
try std.testing.expectEqual(@as(usize, 0), ctx.err_len);
30013010
}
30023011

3012+
test "SQLite lease release consumes connection when idle growth is OOM" {
3013+
var failing = std.testing.FailingAllocator.init(std.testing.allocator, .{});
3014+
var pool = try Pool.init(failing.allocator(), std.testing.io, .{
3015+
.max_open = 1,
3016+
.max_idle = 1,
3017+
});
3018+
defer pool.deinit();
3019+
3020+
var lease = try pool.acquire();
3021+
try (try lease.conn()).ping();
3022+
failing.fail_index = failing.alloc_index;
3023+
try std.testing.expectError(error.OutOfMemory, lease.release());
3024+
try std.testing.expect(failing.has_induced_failure);
3025+
try std.testing.expect(!lease.open);
3026+
try std.testing.expectEqual(@as(usize, 0), pool.stats().open);
3027+
try std.testing.expectEqual(@as(usize, 0), pool.stats().leased);
3028+
3029+
failing.fail_index = std.math.maxInt(usize);
3030+
try pool.ping();
3031+
try std.testing.expectEqual(@as(usize, 1), pool.stats().idle);
3032+
}
3033+
3034+
test "SQLite owned pool results unwind when release observes shutdown" {
3035+
const CloseOnQuery = struct {
3036+
pool: *Pool,
3037+
closed: bool = false,
3038+
3039+
fn after(raw: ?*anyopaque, _: core.QueryEnd) void {
3040+
const self: *@This() = @ptrCast(@alignCast(raw.?));
3041+
if (self.closed) return;
3042+
self.closed = true;
3043+
self.pool.deinit();
3044+
}
3045+
3046+
fn hooks(self: *@This()) core.Hooks {
3047+
return .{ .ctx = self, .after_query = after };
3048+
}
3049+
};
3050+
3051+
{
3052+
var pool = try Pool.init(std.testing.allocator, std.testing.io, .{ .max_open = 1 });
3053+
defer pool.deinit();
3054+
_ = try pool.exec("create table release_one (id integer primary key)", &.{});
3055+
_ = try pool.exec("insert into release_one (id) values (1)", &.{});
3056+
var state = CloseOnQuery{ .pool = &pool };
3057+
pool.config.hooks = state.hooks();
3058+
try std.testing.expectError(
3059+
error.PoolClosed,
3060+
pool.queryOne("select id from release_one", &.{}),
3061+
);
3062+
}
3063+
3064+
{
3065+
var pool = try Pool.init(std.testing.allocator, std.testing.io, .{ .max_open = 1 });
3066+
defer pool.deinit();
3067+
_ = try pool.exec("create table release_all (id integer primary key)", &.{});
3068+
_ = try pool.exec("insert into release_all (id) values (1), (2)", &.{});
3069+
var state = CloseOnQuery{ .pool = &pool };
3070+
pool.config.hooks = state.hooks();
3071+
try std.testing.expectError(
3072+
error.PoolClosed,
3073+
pool.queryAll("select id from release_all order by id", &.{}),
3074+
);
3075+
}
3076+
}
3077+
30033078
test "SQLite pool queryOne returns single owned row" {
30043079
var pool = try Pool.init(std.testing.allocator, std.testing.io, .{ .max_open = 2 });
30053080
defer pool.deinit();

‎tests/postgres_live.zig‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,65 @@ test "postgres live: pool queryAllParams collects owned rows" {
419419
try pool.ping();
420420
}
421421

422+
test "postgres live: owned pool results unwind when release observes shutdown" {
423+
var gpa_state: std.heap.DebugAllocator(.{}) = .init;
424+
defer std.debug.assert(gpa_state.deinit() == .ok);
425+
const allocator = gpa_state.allocator();
426+
const url_str = try requireUrl(allocator);
427+
defer allocator.free(url_str);
428+
var config = try pg.parseUrl(allocator, url_str);
429+
defer config.deinit();
430+
431+
const CloseOnQuery = struct {
432+
pool: *pg.Pool,
433+
closed: bool = false,
434+
435+
fn after(raw: ?*anyopaque, _: zsql.QueryEnd) void {
436+
const self: *@This() = @ptrCast(@alignCast(raw.?));
437+
if (self.closed) return;
438+
self.closed = true;
439+
self.pool.deinit();
440+
}
441+
442+
fn hooks(self: *@This()) zsql.Hooks {
443+
return .{ .ctx = self, .after_query = after };
444+
}
445+
};
446+
447+
{
448+
var pool = try pg.Pool.init(allocator, std.testing.io, .{
449+
.database = config,
450+
.max_open = 1,
451+
.max_idle = 1,
452+
});
453+
defer pool.deinit();
454+
var state = CloseOnQuery{ .pool = &pool };
455+
pool.config.hooks = state.hooks();
456+
try std.testing.expectError(
457+
error.PoolClosed,
458+
pool.queryOneParams("select 1::int as id", &.{}),
459+
);
460+
}
461+
462+
{
463+
var pool = try pg.Pool.init(allocator, std.testing.io, .{
464+
.database = config,
465+
.max_open = 1,
466+
.max_idle = 1,
467+
});
468+
defer pool.deinit();
469+
var state = CloseOnQuery{ .pool = &pool };
470+
pool.config.hooks = state.hooks();
471+
try std.testing.expectError(
472+
error.PoolClosed,
473+
pool.queryAllParams(
474+
"select id from (values (1), (2)) as rows(id) order by id",
475+
&.{},
476+
),
477+
);
478+
}
479+
}
480+
422481
test "postgres live: pool withTx commits and rolls back" {
423482
var gpa_state: std.heap.DebugAllocator(.{}) = .init;
424483
defer _ = gpa_state.deinit();

0 commit comments

Comments
 (0)