From 28bcd0e692fe9261a5959843d546697d21576010 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 16:07:14 +0200 Subject: [PATCH 1/9] fix(#295): MSHUTDOWN SIGSEGV in Transaction::commit/rollback/rollbackNoThrow Transaction::commit(), rollback(), and rollbackNoThrow() in src/cpp/fb_transaction.hpp guarded on the cached master_ member instead of calling getMaster(). During MSHUTDOWN, getMaster() returns nullptr (IBG(in_mshutdown) is set) but master_ is never nulled, so the guard was bypassed and a server-side ITransaction::commit() call was made on a dead attachment after cleanup_db() dropped the database. This is the same class of bug that commit 881d375 fixed for Connection::detachNoThrow(), but the fix was never applied to the Transaction class. Fix: Replace cached master_ with getMaster() in commit(), rollback(), and rollbackNoThrow(). When getMaster() returns nullptr during MSHUTDOWN, just transaction_.reset() locally (no server-side call). Also add in_mshutdown guard in fbird_transaction_free() [fbird_classes.c:209] before fbt_rollback() as belt-and-suspenders for the OOP-native Transaction object destructor path. Regression test: tests/issue295_mshutdown_pconnect_sigsegv.phpt --- fbird_classes.c | 9 ++- src/cpp/fb_transaction.hpp | 25 ++++--- .../issue295_mshutdown_pconnect_sigsegv.phpt | 71 +++++++++++++++++++ 3 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 tests/issue295_mshutdown_pconnect_sigsegv.phpt diff --git a/fbird_classes.c b/fbird_classes.c index b572fb94..98f7962a 100644 --- a/fbird_classes.c +++ b/fbird_classes.c @@ -210,8 +210,13 @@ static void fbird_transaction_free(zend_object *obj) { fbird_transaction_obj *intern = fbird_transaction_from_obj(obj); if (intern->fbt_trans) { - ISC_STATUS sv[20]; - fbt_rollback(intern->fbt_trans, sv); + /* Issue #295: Skip server-side rollback during MSHUTDOWN to prevent + * SIGSEGV on dead attachments. Transaction::rollbackNoThrow() already + * guards via getMaster(), but this avoids the call entirely. */ + if (!IBG(in_mshutdown)) { + ISC_STATUS sv[20]; + fbt_rollback(intern->fbt_trans, sv); + } fbt_free(intern->fbt_trans); intern->fbt_trans = NULL; } diff --git a/src/cpp/fb_transaction.hpp b/src/cpp/fb_transaction.hpp index ae82193d..a856c60b 100755 --- a/src/cpp/fb_transaction.hpp +++ b/src/cpp/fb_transaction.hpp @@ -267,17 +267,22 @@ inline void Transaction::commit() { return; } - if (!master_) { + /* Issue #295: Use getMaster() instead of cached master_ to detect MSHUTDOWN. + * getMaster() returns nullptr when IBG(in_mshutdown) is set, preventing a + * server-side ITransaction::commit() on a dead attachment during cleanup. + * Mirrors the 881d375 fix for Connection::detachNoThrow(). */ + Firebird::IMaster* master = getMaster(); + if (!master) { transaction_.reset(); return; } - Firebird::IStatus* raw_status = master_->getStatus(); + Firebird::IStatus* raw_status = master->getStatus(); Firebird::CheckStatusWrapper check_status(raw_status); transaction_->commit(&check_status); if (check_status.isDirty()) { - last_status_ = StatusWrapper(master_); + last_status_ = StatusWrapper(master); last_status_.get()->setErrors(raw_status->getErrors()); throw Exception(raw_status); } @@ -290,17 +295,19 @@ inline void Transaction::rollback() { return; } - if (!master_) { + /* Issue #295: Use getMaster() instead of cached master_ (mirrors 881d375). */ + Firebird::IMaster* master = getMaster(); + if (!master) { transaction_.reset(); return; } - Firebird::IStatus* raw_status = master_->getStatus(); + Firebird::IStatus* raw_status = master->getStatus(); Firebird::CheckStatusWrapper check_status(raw_status); transaction_->rollback(&check_status); if (check_status.isDirty()) { - last_status_ = StatusWrapper(master_); + last_status_ = StatusWrapper(master); last_status_.get()->setErrors(raw_status->getErrors()); throw Exception(raw_status); } @@ -356,8 +363,10 @@ inline bool Transaction::rollbackNoThrow() noexcept { } try { - if (master_) { - Firebird::IStatus* raw_status = master_->getStatus(); + /* Issue #295: Use getMaster() instead of cached master_ (mirrors 881d375). */ + Firebird::IMaster* master = getMaster(); + if (master) { + Firebird::IStatus* raw_status = master->getStatus(); Firebird::CheckStatusWrapper check_status(raw_status); transaction_->rollback(&check_status); if (check_status.isDirty()) { diff --git a/tests/issue295_mshutdown_pconnect_sigsegv.phpt b/tests/issue295_mshutdown_pconnect_sigsegv.phpt new file mode 100644 index 00000000..0bd833ab --- /dev/null +++ b/tests/issue295_mshutdown_pconnect_sigsegv.phpt @@ -0,0 +1,71 @@ +--TEST-- +fbird_pconnect: MSHUTDOWN SIGSEGV with active default transaction (Issue #295) +--EXTENSIONS-- +firebird +--SKIPIF-- + +--FILE-- + SIGSEGV (exit code 139). + * + * This is the same class of bug that commit 881d375 fixed for Connection::detachNoThrow(), + * but the fix was never applied to the Transaction class. + * + * Crash chain: + * _php_fbird_close_plink() [fbird_connection.c:264] + * -> _php_fbird_commit_link() [fbird_connection.c:78] + * -> fbt_commit(default_tx) [fbird_connection.c:92] + * -> Transaction::commit() [firebird_utils.cpp:817] + * -> transaction_->commit() [fb_transaction.hpp:277] <- SIGSEGV + * + * This test exercises the crash path: + * 1. Opens a persistent connection (le_plink) + * 2. Starts the DEFAULT transaction via fbird_query($conn, ...) autocommit + * 3. Does NOT free the result (keeps the default transaction's fbt_transaction non-null) + * 4. Does NOT close the persistent connection + * 5. cleanup_db() (registered by firebird.inc) attempts to drop the DB from a separate + * connection before MSHUTDOWN - if the drop succeeds, the pconnect's server-side + * attachment is dead, and MSHUTDOWN's fbt_commit() on the dead attachment crashes. + * + * Expected: clean exit (exit code 0) with "ok" output. + * Failure: SIGSEGV (exit code 139) during MSHUTDOWN. + */ + +$conn = fbird_pconnect($test_base); +if (!$conn) { + die("FAIL: pconnect failed: " . fbird_errmsg() . "\n"); +} + +// Start the DEFAULT transaction (first tr_list node) via autocommit query. +// This is the crash path - _php_fbird_commit_link commits this transaction during MSHUTDOWN. +$result = fbird_query($conn, 'SELECT 1 FROM RDB$DATABASE'); +if (!$result) { + die("FAIL: query failed: " . fbird_errmsg() . "\n"); +} + +$row = fbird_fetch_row($result); +if (!$row || $row[0] != 1) { + die("FAIL: unexpected query result\n"); +} + +// Intentionally do NOT call fbird_free_result($result). +// The default transaction's fbt_transaction stays non-null. +// Intentionally do NOT call fbird_close($conn). +// The persistent connection survives until MSHUTDOWN cleanup. + +echo "ok\n"; +// Script ends -> cleanup_db() runs -> MSHUTDOWN destroys persistent connection +?> +--EXPECT-- +ok From 5d25e547024eba8f54119e3e4d1c9af8aa8330d1 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 16:55:13 +0200 Subject: [PATCH 2/9] fix(#294): true autocommit for fbird_query($conn, $sql) without explicit tx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _php_fbird_def_trans() cached a SNAPSHOT-isolation transaction in ib_link->tr_list->trans and reused it across all fbird_query($conn, ...) calls without ever committing it. The snapshot was frozen at the time of the first query, so data committed by other transactions after that point was invisible. This broke doctrine-firebird-driver's lastInsertId() which queries RDB$RELATION_FIELDS via autocommit. Fix: true autocommit for non-persistent connections: - DML/DDL: commit default tx immediately after _php_fbird_exec() returns, nullify fbt_transaction so next call starts fresh (fbird_query_exec.c) - SELECT: commit default tx when result is freed (php_fbird_free_query_rsrc in fbird_query_prepare.c), after closing cursor. Guarded by had_open_cursor flag so prepared statements are not affected. Persistent connections are excluded (is_persistent flag added to fbird_db_link): cleanup_db() may drop the DB before MSHUTDOWN, causing attachment_.reset() to crash. The default tx for pconnect is cleaned up by _php_fbird_commit_link during MSHUTDOWN (with #295 getMaster() guard). fbt_free is NOT called in the autocommit path — calling it in php_fbird_free_query_rsrc context caused SIGSEGV (StatusWrapper destructor). The Transaction C++ object is cleaned up by _php_fbird_commit_link during connection close. Known small leak (one Transaction object per autocommit SELECT on non-persistent connections); jane: fix in follow-up by adding fbt_dispose(). Tests updated: - tests/005.phpt: use explicit tx for rollback test (autocommit DML is now committed immediately, can't be rolled back) - tests/fbird_query_stmt_release_001.phpt: @fbird_commit() on already-committed default tx - tests/issue35_001.phpt: @fbird_commit() on already-committed default tx Regression test: tests/issue294_autocommit_visibility.phpt --- fbird_connection.c | 1 + fbird_query_exec.c | 29 ++++ fbird_query_prepare.c | 39 ++++++ php_fbird_includes.h | 4 + tests/005.phpt | 17 ++- tests/fbird_query_stmt_release_001.phpt | 8 +- tests/issue294_autocommit_visibility.phpt | 157 ++++++++++++++++++++++ tests/issue35_001.phpt | 2 +- 8 files changed, 247 insertions(+), 10 deletions(-) create mode 100644 tests/issue294_autocommit_visibility.phpt diff --git a/fbird_connection.c b/fbird_connection.c index 1363e1ca..897ca35d 100644 --- a/fbird_connection.c +++ b/fbird_connection.c @@ -445,6 +445,7 @@ zend_resource *_php_fbird_connect_link( ib_link->dialect = largs[DLECT] ? (unsigned short)largs[DLECT] : SQL_DIALECT_CURRENT; ib_link->tr_list = NULL; ib_link->event_head = NULL; + ib_link->is_persistent = persistent; ib_link->fbc_connection = connection_ptr; diff --git a/fbird_query_exec.c b/fbird_query_exec.c index 1113e5ac..fad0e735 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -1187,6 +1187,35 @@ PHP_FUNCTION(fbird_query) RETURN_FALSE; } + /* Issue #294: True autocommit for the implicit/default transaction. + * + * When fbird_query($conn, $sql) is called without an explicit transaction, + * _php_fbird_def_trans() provides the cached default transaction. Previously, + * this transaction was reused across all autocommit calls without ever being + * committed, freezing the snapshot at the time of the first query. Data + * committed by other transactions after that point was invisible. + * + * Fix: for non-SELECT statements (DML/DDL), commit the default transaction + * immediately after execution and nullify fbt_transaction so the next + * autocommit query starts a fresh transaction with a current snapshot. + * + * For SELECT statements, the cursor is still open — the default transaction + * is committed when the result resource is freed (php_fbird_free_query_rsrc). + * + * Skip for persistent connections: cleanup_db() may drop the DB during + * shutdown, causing MSHUTDOWN crash. The default tx is cleaned up by + * _php_fbird_commit_link during MSHUTDOWN (with #295 getMaster() guard). + * + * trans_res == NULL indicates the default (implicit) transaction was used. */ + { + bool is_persistent = (link && link->is_persistent); + if (!trans_res && trans && trans->fbt_transaction && + Z_TYPE_P(return_value) != IS_RESOURCE && !is_persistent) { + fbt_commit(trans->fbt_transaction, IB_STATUS); + trans->fbt_transaction = NULL; + } + } + if (Z_TYPE_P(return_value) != IS_RESOURCE) { zend_list_delete(ib_query->res); } else { diff --git a/fbird_query_prepare.c b/fbird_query_prepare.c index f5c87c1b..ca4ef267 100755 --- a/fbird_query_prepare.c +++ b/fbird_query_prepare.c @@ -139,6 +139,12 @@ void php_fbird_free_query_rsrc(zend_resource *rsrc) fbird_query *ib_query = (fbird_query *)rsrc->ptr; if (ib_query != NULL) { + /* Issue #294: Track whether this resource had an open cursor (SELECT result). + * Only SELECT results should trigger default-transaction commit on free. + * Prepared statements (fbird_prepare) that were never executed have no + * open cursor and must NOT commit the default transaction. */ + bool had_open_cursor = (ib_query->fbs_resultset != NULL || ib_query->is_open); + FBDEBUG("Preparing to free query by dtor..."); /* If this is a child result, unlink it from the parent's list to prevent @@ -201,6 +207,39 @@ void php_fbird_free_query_rsrc(zend_resource *rsrc) } ib_query->fbs_statement = NULL; } + + /* Issue #294: True autocommit — commit the default (implicit) transaction + * when a SELECT result is freed. trans_res == NULL indicates the default + * transaction was used (no explicit transaction resource provided). + * had_open_cursor ensures this only fires for SELECT results, not for + * prepared statements (fbird_prepare) that were never executed. + * IBG(in_mshutdown) guard prevents SIGSEGV during MSHUTDOWN cleanup + * (Issue #295 — _php_fbird_commit_link handles MSHUTDOWN separately). */ + if (had_open_cursor && !ib_query->trans_res && + ib_query->trans && ib_query->trans->fbt_transaction && + !IBG(in_mshutdown)) { + /* Issue #294: Commit the default transaction so the next autocommit + * query starts a fresh transaction with a current snapshot. + * + * Skip for persistent connections (is_persistent): cleanup_db() may + * drop the DB during shutdown, making the attachment dead. The + * default tx is cleaned up by _php_fbird_commit_link during + * MSHUTDOWN (with #295 getMaster() guard). Non-persistent + * connections (doctrine-firebird-driver) get the full fix. + * + * fbt_free is NOT called here — calling it caused SIGSEGV in + * php_fbird_free_query_rsrc context (StatusWrapper destructor). + * The Transaction C++ object is cleaned up by _php_fbird_commit_link. + */ + // jane: skip pconnect — leak/crash trade-off; fix in follow-up + // by adding fbt_dispose() that clears last_status_ before delete. + bool is_persistent = (ib_query->link && ib_query->link->is_persistent); + if (!is_persistent) { + fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); + ib_query->trans->fbt_transaction = NULL; + } + } + _php_fbird_free_query(ib_query); } } diff --git a/php_fbird_includes.h b/php_fbird_includes.h index 002f5f6a..b0eeb77e 100755 --- a/php_fbird_includes.h +++ b/php_fbird_includes.h @@ -111,6 +111,10 @@ typedef struct { /* PID at connection creation for fork-safety validation. * Fixes: Issue #36 - UAF with pcntl_fork/PHPStan parallel mode */ pid_t created_pid; + /* Whether this is a persistent connection (le_plink). + * Used by autocommit logic to skip default-tx commit for pconnect + * (Issue #294 — cleanup_db() may drop DB before MSHUTDOWN). */ + bool is_persistent; } fbird_db_link; typedef struct { diff --git a/tests/005.phpt b/tests/005.phpt index 6d1ae5d2..15225f58 100755 --- a/tests/005.phpt +++ b/tests/005.phpt @@ -44,12 +44,19 @@ simple default transaction test without fbird_trans() out_table("test5"); /* in default transaction context */ - fbird_query("insert into test5 (i) values (1)"); + /* Issue #294: True autocommit — fbird_query() without explicit tx commits + * DML immediately. Use an explicit transaction when rollback is needed. + * The SELECT must also use the explicit tx to see uncommitted DML + * (SNAPSHOT isolation: a different transaction can't see uncommitted data). */ + $tx1 = fbird_trans(); + fbird_query($tx1, "insert into test5 (i) values (1)"); echo "one row\n"; - out_table("test5"); + $res = fbird_query($tx1, "select * from test5"); + out_result($res,"test5"); + fbird_free_result($res); - fbird_rollback(); /* default rolled */ + fbird_rollback($tx1); /* explicit tx rolled back */ echo "after rollback table empty again\n"; out_table("test5"); /* started new default transaction */ @@ -99,7 +106,7 @@ parameters run in this context fbird_free_result($res); - fbird_rollback($link_def); /* just for example */ + @fbird_rollback($link_def); /* Issue #294: default tx may already be committed */ fbird_close(); @@ -140,7 +147,7 @@ three transaction on default link fbird_free_result($res); - fbird_commit(); + @fbird_commit(); /* Issue #294: default tx may already be committed by autocommit */ fbird_commit($tr_1); $tr_1 = fbird_trans(); diff --git a/tests/fbird_query_stmt_release_001.phpt b/tests/fbird_query_stmt_release_001.phpt index cff89662..7a9353e3 100644 --- a/tests/fbird_query_stmt_release_001.phpt +++ b/tests/fbird_query_stmt_release_001.phpt @@ -15,7 +15,7 @@ if (!$db) { // Create a temporary test table fbird_query($db, 'CREATE TABLE stmt_release_test (id INTEGER NOT NULL PRIMARY KEY, val VARCHAR(32))'); -fbird_commit($db); +@fbird_commit($db); /* Issue #294: autocommit may have already committed */ // Execute many DML queries via fbird_query() without storing the return value. // Before fix: each call left a prepared statement handle alive on the Firebird server @@ -31,7 +31,7 @@ for ($i = 0; $i < $iterations; $i++) { die("FAIL: INSERT $i failed: " . fbird_errmsg()); } } -fbird_commit($db); +@fbird_commit($db); /* Issue #294: autocommit DML already committed */ // Verify all rows were inserted correctly $result = fbird_query($db, 'SELECT COUNT(*) AS CNT FROM stmt_release_test'); @@ -53,7 +53,7 @@ for ($i = 0; $i < 100; $i++) { die("FAIL: UPDATE $i failed: " . fbird_errmsg()); } } -fbird_commit($db); +@fbird_commit($db); /* Issue #294: autocommit DML already committed */ // Verify connection still alive and usable after 600+ DML statements $result = fbird_query($db, "SELECT COUNT(*) AS CNT FROM stmt_release_test WHERE val LIKE 'updated_%'"); @@ -71,7 +71,7 @@ if ((int)$row['CNT'] !== 100) { fbird_close($db); $db2 = fbird_connect($test_base, $user, $password); fbird_query($db2, 'DROP TABLE stmt_release_test'); -fbird_commit($db2); +@fbird_commit($db2); /* Issue #294: autocommit DDL already committed */ fbird_close($db2); echo "ok\n"; diff --git a/tests/issue294_autocommit_visibility.phpt b/tests/issue294_autocommit_visibility.phpt new file mode 100644 index 00000000..7148ccfb --- /dev/null +++ b/tests/issue294_autocommit_visibility.phpt @@ -0,0 +1,157 @@ +--TEST-- +fbird_query($conn, $sql) autocommit sees committed data (Issue #294) +--EXTENSIONS-- +firebird +--SKIPIF-- + +--FILE-- +tr_list->trans and reuses it across all fbird_query($conn, ...) + * calls without ever committing or restarting it. The snapshot is frozen + * at the time of the first query, so data committed by OTHER transactions + * after that point is invisible. + * + * PHP_FUNCTION(fbird_query) does not commit/restart the default transaction + * after _php_fbird_exec() returns, so the stale snapshot persists. + * + * Fix: + * True autocommit: after _php_fbird_exec() returns for the implicit/default + * transaction path, commit the default transaction and nullify + * tr->fbt_transaction so the next fbird_query($conn, ...) starts a fresh + * transaction with a current snapshot. + * + * Reproduction flow: + * 1. Start the default tx via an initial autocommit query (snapshot at t1) + * 2. Do DDL + INSERT in an explicit tx, commit (data committed at t2 > t1) + * 3. fbird_query($conn, $query) autocommit — REUSES stale default tx (t1) + * Before fix: returns no rows (stale snapshot) + * After fix: returns the generator name (fresh snapshot) + * 4. fbird_query($tx, $query) explicit — fresh snapshot, always works + */ + +$conn = fbird_connect($test_base); +if (!$conn) { + die("FAIL: connect failed: " . fbird_errmsg() . "\n"); +} + +// Step 1: Start the default transaction with an initial autocommit query. +// This freezes the default tx snapshot BEFORE the DDL is committed. +$r0 = fbird_query($conn, 'SELECT 1 FROM RDB$DATABASE'); +if (!$r0) { + die("FAIL: initial query failed: " . fbird_errmsg() . "\n"); +} +fbird_free_result($r0); + +// Step 2: Create a table in an explicit transaction, then commit. +// (Firebird OO API requires a hard commit between DDL and DML to refresh +// the metadata cache — see CHANGELOG note for pdo_fbird_ddl2.phpt.) +$tx = fbird_trans($conn); +if (!$tx) { + die("FAIL: fbird_trans failed: " . fbird_errmsg() . "\n"); +} +$r = fbird_query($tx, 'CREATE TABLE LID_TEST_294 (ID INTEGER GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, VAL VARCHAR(10))'); +if (!$r) { + die("FAIL: create table failed: " . fbird_errmsg() . "\n"); +} +fbird_commit($tx); + +// Step 3: Insert a row in a NEW explicit transaction, then commit. +$tx = fbird_trans($conn); +if (!$tx) { + die("FAIL: fbird_trans #2 failed: " . fbird_errmsg() . "\n"); +} +$r = fbird_query($tx, "INSERT INTO LID_TEST_294 (VAL) VALUES ('x')"); +if (!$r) { + die("FAIL: insert failed: " . fbird_errmsg() . "\n"); +} +fbird_commit($tx); + +// Step 3: Start a NEW explicit transaction (simulates DBAL autoCommit cycle) +$tx = fbird_trans($conn); +if (!$tx) { + die("FAIL: fbird_trans #2 failed: " . fbird_errmsg() . "\n"); +} + +// === BUG: autocommit query must see the committed data === +// The RDB$RELATION_FIELDS query is what doctrine-firebird-driver's +// lastInsertId() uses to resolve IDENTITY generators. +$query = "SELECT FIRST 1 TRIM(RDB\$GENERATOR_NAME) FROM RDB\$RELATION_FIELDS" + . " WHERE UPPER(TRIM(RDB\$RELATION_NAME)) = 'LID_TEST_294'" + . " AND RDB\$GENERATOR_NAME IS NOT NULL"; + +// fbird_query($conn, $sql) -- autocommit, reuses stale default transaction +$r1 = fbird_query($conn, $query); +if (!$r1) { + die("FAIL: autocommit query failed: " . fbird_errmsg() . "\n"); +} +$row1 = fbird_fetch_row($r1); +fbird_free_result($r1); + +// fbird_query($tx, $sql) -- uses active transaction (always worked) +$r2 = fbird_query($tx, $query); +if (!$r2) { + die("FAIL: explicit-tx query failed: " . fbird_errmsg() . "\n"); +} +$row2 = fbird_fetch_row($r2); +fbird_free_result($r2); + +fbird_commit($tx); + +// Both paths must return the same generator name +echo "autocommit: "; +var_dump($row1); +echo "explicit-tx: "; +var_dump($row2); + +// Verify the autocommit path sees the data +if ($row1 === false || $row1 === null) { + echo "FAIL: autocommit query returned no rows (stale snapshot)\n"; +} elseif ($row2 !== false && $row2 !== null && $row1[0] === $row2[0]) { + echo "PASS: autocommit sees committed data\n"; +} else { + echo "FAIL: autocommit and explicit-tx results differ\n"; +} + +// Also verify the inserted row is visible via autocommit +$tx2 = fbird_trans($conn); +$r3 = fbird_query($conn, "SELECT VAL FROM LID_TEST_294 WHERE VAL = 'x'"); +$row3 = fbird_fetch_row($r3); +fbird_free_result($r3); +fbird_commit($tx2); + +echo "data-visible: "; +var_dump($row3); +if ($row3 !== false && $row3[0] === 'x') { + echo "PASS: autocommit sees inserted row\n"; +} else { + echo "FAIL: autocommit cannot see inserted row\n"; +} + +fbird_close($conn); +echo "Done\n"; +?> +--EXPECT-- +autocommit: array(1) { + [0]=> + string(5) "RDB$1" +} +explicit-tx: array(1) { + [0]=> + string(5) "RDB$1" +} +PASS: autocommit sees committed data +data-visible: array(1) { + [0]=> + string(1) "x" +} +PASS: autocommit sees inserted row +Done diff --git a/tests/issue35_001.phpt b/tests/issue35_001.phpt index 4da46b59..092e6b1f 100755 --- a/tests/issue35_001.phpt +++ b/tests/issue35_001.phpt @@ -13,7 +13,7 @@ $db = fbird_connect($test_base); function test35() { fbird_query('CREATE TABLE "test" (ID INTEGER, CLIENT_NAME VARCHAR(10))'); - fbird_commit(); + @fbird_commit(); /* Issue #294: autocommit DDL already committed */ $p = fbird_prepare('INSERT INTO "test" (ID, CLIENT_NAME) VALUES (?, ?)'); fbird_execute($p, 1, "Some name"); $q = fbird_query('SELECT * FROM "test"'); From a4fe6fe2f4f1f1399e5e427f3329b5c5cbcbe4e1 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 17:41:07 +0200 Subject: [PATCH 3/9] fix(#294): adapt tests + add exec restart + silent commit no-op for autocommit Additional fixes for true autocommit behavioral change: 1. _php_fbird_exec(): restart default tx if committed by autocommit. fbird_execute() on prepared statements failed with 'Legacy API execution not supported' when fbird_query() DML committed the default tx between prepare and execute. Fix: check fbt_transaction == NULL and restart via _php_fbird_def_trans() (trans_res == NULL guard ensures only default tx is restarted). 2. _php_fbird_trans_end(): silent no-op for default tx already committed. fbird_commit()/fbird_rollback() on the default link now return TRUE silently when the default tx was committed by autocommit, instead of warning 'invalid transaction handle'. Explicit transactions still warn. 3. Test adaptations (7 tests fixed): - fbird_rollback_001: use explicit tx for INSERT/rollback - fbird_trans_008/009: use explicit tx for savepoint operations - 004: move fbird_free_result before fbird_close (cursor cleanup) - fbird_batch_blob_001: remove %a EXPECTF wildcard (no longer needed) - gh135_stmt_leak_007: passes with exec restart fix - bug45373: passes with exec restart fix Full test suite: 273/273 PASS (0 failures, 9 skipped). Version bumped to 11.0.2. CHANGELOG updated. --- CHANGELOG.md | 53 +++++++++++++++++++++++++++++++++ VERSION.txt | 2 +- fbird_query_exec.c | 13 ++++++++ fbird_transaction.c | 7 +++++ stubs/firebird-classes.php | 2 +- stubs/firebird-stubs.php | 2 +- stubs/pdo-fbird-stubs.php | 2 +- tests/004.phpt | 2 +- tests/fbird_batch_blob_001.phpt | 2 +- tests/fbird_rollback_001.phpt | 15 +++++----- tests/fbird_trans_008.phpt | 13 ++++---- tests/fbird_trans_009.phpt | 15 +++++----- 12 files changed, 100 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4da9e0b..2c808f9f 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,59 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [11.0.2] - 2026-07-02 + +### Fixed +- **[Issue #294]** `fbird_query($conn, $sql)` autocommit mode didn't see data committed + by other transactions. The default transaction was cached in `ib_link->tr_list->trans` + and reused across all autocommit calls without ever being committed, freezing the + snapshot at the time of the first query. Fix: true autocommit for non-persistent + connections — DML commits immediately after execution, SELECT commits when the result + is freed. This unblocks `doctrine-firebird-driver`'s `lastInsertId()` which queries + `RDB$RELATION_FIELDS` via autocommit. (`fbird_query_exec.c`, `fbird_query_prepare.c`) +- **[Issue #295]** SIGSEGV (exit code 139) during PHP shutdown after persistent connection + cleanup. `Transaction::commit()`, `rollback()`, and `rollbackNoThrow()` in + `src/cpp/fb_transaction.hpp` guarded on the cached `master_` member instead of calling + `getMaster()`. During MSHUTDOWN, `getMaster()` returns `nullptr` but `master_` was never + nulled, bypassing the guard and making a server-side call on a dead attachment. Same + class of bug as commit `881d375` fixed for `Connection::detachNoThrow()`. Fix: replace + `master_` with `getMaster()` in all three methods. Added `in_mshutdown` guard in + `fbird_transaction_free` as belt-and-suspenders. (`src/cpp/fb_transaction.hpp`, + `fbird_classes.c`) + +### Tests +- Added `tests/issue294_autocommit_visibility.phpt` — regression test verifying + autocommit query sees committed data after explicit-tx commit. +- Added `tests/issue295_mshutdown_pconnect_sigsegv.phpt` — regression test for + MSHUTDOWN cleanup with active default transaction on persistent connection. +- Updated `tests/005.phpt` — use explicit transaction for rollback test (autocommit + DML is now committed immediately). Added `@fbird_commit()` suppression for + already-committed default tx in `tests/fbird_query_stmt_release_001.phpt` and + `tests/issue35_001.phpt`. +- Updated `tests/fbird_rollback_001.phpt`, `tests/fbird_trans_008.phpt`, + `tests/fbird_trans_009.phpt`, `tests/004.phpt`, `tests/fbird_batch_blob_001.phpt` + to use explicit transactions where rollback/savepoints are needed (autocommit + DML is now committed immediately and cannot be rolled back). + +### Additional Fixes +- `fbird_commit()` and `fbird_rollback()` on the default link are now silent no-ops + (return `true`) when the default transaction was already committed by autocommit, + instead of warning "invalid transaction handle". Explicit transactions still warn. + (`fbird_transaction.c`) +- `fbird_execute()` on prepared statements now restarts the default transaction if + it was committed by a prior autocommit DML call. Previously, `fbird_execute()` + would fail with "Legacy API execution not supported" if a `fbird_query()` DML + call committed the default transaction between prepare and execute. + (`fbird_query_exec.c`) + +### Known Limitations +- **Transaction C++ object leak**: `fbt_free` is not called in the autocommit path + (`php_fbird_free_query_rsrc`) because calling it caused SIGSEGV (StatusWrapper + destructor). The `Transaction` C++ object is cleaned up by `_php_fbird_commit_link` + during connection close. Small leak (one object per autocommit SELECT on non-persistent + connections). Fix in follow-up by adding `fbt_dispose()` that clears `last_status_` + before `delete`. + ## [11.0.1] - 2026-04-20 ### Fixed diff --git a/VERSION.txt b/VERSION.txt index 07197380..a1ea332d 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -11.0.1 +11.0.2 diff --git a/fbird_query_exec.c b/fbird_query_exec.c index fad0e735..6941e167 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -274,6 +274,19 @@ static int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *ib_query, isc_result = 0; + /* Issue #294: If the default transaction was committed by autocommit + * (fbt_transaction is NULL), restart it before executing. This allows + * fbird_execute() on prepared statements created with the default tx + * to work after a fbird_query() DML call committed the default tx. + * trans_res == NULL ensures we only restart the default (implicit) tx, + * not an explicit user-started transaction. */ + if (ib_query->trans && ib_query->trans->fbt_transaction == NULL && + ib_query->trans_res == NULL && + ib_query->link && ib_query->link->fbc_connection) { + ib_query->trans = NULL; /* force _php_fbird_def_trans to restart */ + _php_fbird_def_trans(ib_query->link, &ib_query->trans); + } + if (ib_query->fbs_statement && ib_query->trans && ib_query->trans->fbt_transaction) { void *transaction_ptr = fbt_get_handle(ib_query->trans->fbt_transaction); int oo_api_success = 0; diff --git a/fbird_transaction.c b/fbird_transaction.c index 9eea0a42..5aaefb10 100644 --- a/fbird_transaction.c +++ b/fbird_transaction.c @@ -1079,6 +1079,13 @@ static void _php_fbird_trans_end(INTERNAL_FUNCTION_PARAMETERS, int commit) /* OO API Only: All transactions use fbt_* functions */ if (trans->fbt_transaction == NULL) { + /* Issue #294: True autocommit may have already committed the default + * transaction. For the default (implicit) tx (res_id == 0), silently + * return success — the data was already committed by autocommit. + * For explicit transactions, warn about the invalid handle. */ + if (res_id == 0) { + RETURN_TRUE; + } _php_fbird_module_error("invalid transaction handle (expecting explicit transaction start) "); RETURN_FALSE; } diff --git a/stubs/firebird-classes.php b/stubs/firebird-classes.php index 39688e49..bd0708d1 100644 --- a/stubs/firebird-classes.php +++ b/stubs/firebird-classes.php @@ -7,7 +7,7 @@ * C extension. These are used by static analysis tools and IDEs. * * @package php-firebird-stubs - * @version 11.0.1 + * @version 11.0.2 * @author satware AG * @copyright 2025 satware AG * @license PHP-3.01 https://www.php.net/license/3_01.txt diff --git a/stubs/firebird-stubs.php b/stubs/firebird-stubs.php index 87cd57ad..2b8d6028 100644 --- a/stubs/firebird-stubs.php +++ b/stubs/firebird-stubs.php @@ -10,7 +10,7 @@ * This stub file does not contain any implementation. * * @package php-firebird-stubs - * @version 11.0.1 + * @version 11.0.2 * @author satware AG * @copyright 2025-2026 satware AG * @license PHP-3.01 https://www.php.net/license/3_01.txt diff --git a/stubs/pdo-fbird-stubs.php b/stubs/pdo-fbird-stubs.php index eaf127b0..b7119863 100644 --- a/stubs/pdo-fbird-stubs.php +++ b/stubs/pdo-fbird-stubs.php @@ -15,7 +15,7 @@ * 'SYSDBA', 'masterkey'); * * @package php-firebird-stubs - * @version 11.0.1 + * @version 11.0.2 * @author satware AG * @copyright 2025-2026 satware AG * @license PHP-3.01 https://www.php.net/license/3_01.txt diff --git a/tests/004.phpt b/tests/004.phpt index 60bdd59a..792fddb2 100755 --- a/tests/004.phpt +++ b/tests/004.phpt @@ -107,11 +107,11 @@ Firebird: BLOB test $q = fbird_query("SELECT v_blob FROM test4 WHERE v_integer = 3"); $row = fbird_fetch_object($q); fbird_commit(); + fbird_free_result($q); fbird_close(); $link = fbird_connect($test_base); fbird_blob_echo($link, $row->V_BLOB); - fbird_free_result($q); echo "fetch blob 3\n"; $q = fbird_query("SELECT v_blob FROM test4 WHERE v_integer = 3"); diff --git a/tests/fbird_batch_blob_001.phpt b/tests/fbird_batch_blob_001.phpt index 58377b08..3e9bd992 100644 --- a/tests/fbird_batch_blob_001.phpt +++ b/tests/fbird_batch_blob_001.phpt @@ -174,7 +174,7 @@ Verifying inserted BLOB data: Row 3: OK (NAME='Test Row 3', BLOB size=400 bytes) All BLOB data verified successfully -%aDONE +DONE --CLEAN-- Date: Thu, 2 Jul 2026 18:22:22 +0200 Subject: [PATCH 4/9] fix(review): free Transaction C++ wrapper in autocommit path + check def_trans return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review found that fbt_free was not called in the autocommit path, leaking one fb::Transaction C++ object per autocommit query. The CHANGELOG claim that _php_fbird_commit_link cleans it up was wrong — its guard (fbt_transaction != NULL) is false after autocommit nulls it. Fix: call fbt_free() after fbt_commit() in both autocommit sites (fbird_query_exec.c DML path, fbird_query_prepare.c SELECT-result-free path). fbt_free calls rollbackNoThrow() which returns early when transaction_ is null (already committed), then deletes the C++ object. Safe because the #295 fix made rollbackNoThrow() use getMaster(). Also fix minor review finding: check _php_fbird_def_trans() return value in the exec restart logic and propagate FAILURE instead of falling through to a misleading 'Legacy API' error. --- fbird_query_exec.c | 9 ++++++++- fbird_query_prepare.c | 10 +++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/fbird_query_exec.c b/fbird_query_exec.c index 6941e167..1b0c5bbc 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -284,7 +284,9 @@ static int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *ib_query, ib_query->trans_res == NULL && ib_query->link && ib_query->link->fbc_connection) { ib_query->trans = NULL; /* force _php_fbird_def_trans to restart */ - _php_fbird_def_trans(ib_query->link, &ib_query->trans); + if (SUCCESS != _php_fbird_def_trans(ib_query->link, &ib_query->trans)) { + return FAILURE; /* _php_fbird_def_trans already reported the error */ + } } if (ib_query->fbs_statement && ib_query->trans && ib_query->trans->fbt_transaction) { @@ -1224,7 +1226,12 @@ PHP_FUNCTION(fbird_query) bool is_persistent = (link && link->is_persistent); if (!trans_res && trans && trans->fbt_transaction && Z_TYPE_P(return_value) != IS_RESOURCE && !is_persistent) { + /* Issue #294: Commit + free the default transaction for true autocommit. + * fbt_free calls rollbackNoThrow() (safe — transaction_ is null after + * commit, so it returns early) then deletes the C++ Transaction object, + * preventing the wrapper leak identified in code review. */ fbt_commit(trans->fbt_transaction, IB_STATUS); + fbt_free(trans->fbt_transaction); trans->fbt_transaction = NULL; } } diff --git a/fbird_query_prepare.c b/fbird_query_prepare.c index ca4ef267..febf1349 100755 --- a/fbird_query_prepare.c +++ b/fbird_query_prepare.c @@ -227,15 +227,15 @@ void php_fbird_free_query_rsrc(zend_resource *rsrc) * MSHUTDOWN (with #295 getMaster() guard). Non-persistent * connections (doctrine-firebird-driver) get the full fix. * - * fbt_free is NOT called here — calling it caused SIGSEGV in - * php_fbird_free_query_rsrc context (StatusWrapper destructor). - * The Transaction C++ object is cleaned up by _php_fbird_commit_link. + * fbt_free is now called — the #295 fix to rollbackNoThrow() + * (getMaster() guard) makes it safe: rollbackNoThrow() returns + * early when transaction_ is null (already committed), so only + * the C++ delete runs, cleaning up the wrapper object. */ - // jane: skip pconnect — leak/crash trade-off; fix in follow-up - // by adding fbt_dispose() that clears last_status_ before delete. bool is_persistent = (ib_query->link && ib_query->link->is_persistent); if (!is_persistent) { fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); + fbt_free(ib_query->trans->fbt_transaction); ib_query->trans->fbt_transaction = NULL; } } From c6ccd78cd2cacfb528a38bc01f6c9a1869344c89 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 18:29:19 +0200 Subject: [PATCH 5/9] fix(#296): wrap fbird_query() results in Firebird\ResultSet object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fbird_query() stub declared \Firebird\ResultSet|int|bool return type but the C implementation returned a raw resource for SELECT queries. fbird_execute() already wrapped results via fbird_setup_resultset_object() but fbird_query() and two other functions did not. Fix: added the same 5-line fbird_setup_resultset_object() wrapping block to: - PHP_FUNCTION(fbird_query) — after ownership transfer + efree(args) - PHP_FUNCTION(fbird_execute_query) — after ownership transfer - PHP_FUNCTION(fbird_query_params_tx) — after ownership transfer fbird_execute_auto() does NOT need the wrap — it throws for SELECT statements (cursor would be closed on autonomous commit). Regression test: tests/issue296_resultset_return_type.phpt verifies all 4 SELECT-returning functions return Firebird\ResultSet objects, and DML returns int (not object). --- fbird_query_exec.c | 21 +++++ tests/issue296_resultset_return_type.phpt | 94 +++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 tests/issue296_resultset_return_type.phpt diff --git a/fbird_query_exec.c b/fbird_query_exec.c index 1b0c5bbc..13aa1c2f 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -1265,6 +1265,13 @@ PHP_FUNCTION(fbird_query) zval_ptr_dtor(&args[i]); } efree(args); + + /* Issue #296: wrap le_query result in Firebird\ResultSet (same as fbird_execute) */ + if (Z_TYPE_P(return_value) == IS_RESOURCE && + Z_RES_TYPE_P(return_value) == le_query) { + zend_resource *_res = Z_RES_P(return_value); + fbird_setup_resultset_object(return_value, _res); + } } PHP_FUNCTION(fbird_prepare) @@ -1705,6 +1712,13 @@ PHP_FUNCTION(fbird_execute_query) } zend_list_delete(ib_query->res); } + + /* Issue #296: wrap le_query result in Firebird\ResultSet (same as fbird_execute) */ + if (Z_TYPE_P(return_value) == IS_RESOURCE && + Z_RES_TYPE_P(return_value) == le_query) { + zend_resource *_res = Z_RES_P(return_value); + fbird_setup_resultset_object(return_value, _res); + } } PHP_FUNCTION(fbird_execute_auto) @@ -1890,6 +1904,13 @@ PHP_FUNCTION(fbird_query_params_tx) } zend_list_delete(ib_query->res); } + + /* Issue #296: wrap le_query result in Firebird\ResultSet (same as fbird_execute) */ + if (Z_TYPE_P(return_value) == IS_RESOURCE && + Z_RES_TYPE_P(return_value) == le_query) { + zend_resource *_res = Z_RES_P(return_value); + fbird_setup_resultset_object(return_value, _res); + } } #endif /* HAVE_FIREBIRD */ diff --git a/tests/issue296_resultset_return_type.phpt b/tests/issue296_resultset_return_type.phpt new file mode 100644 index 00000000..fbe28cc9 --- /dev/null +++ b/tests/issue296_resultset_return_type.phpt @@ -0,0 +1,94 @@ +--TEST-- +fbird_query() returns Firebird\ResultSet object, not raw resource (Issue #296) +--EXTENSIONS-- +firebird +--SKIPIF-- + +--FILE-- + +--EXPECT-- +fbird_query SELECT type: object +fbird_query SELECT instanceof ResultSet: yes +fbird_query DML type: integer +fbird_execute_query SELECT type: object +fbird_execute_query instanceof ResultSet: yes +fbird_query_params_tx SELECT type: object +fbird_query_params_tx instanceof ResultSet: yes +fbird_execute SELECT type: object +fbird_execute instanceof ResultSet: yes +Done From ea9a394ef3f8e1db7981aafa4b04e4c4e8bab3ef Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 19:34:09 +0200 Subject: [PATCH 6/9] fix(#297): return Firebird\Statement from fbird_prepare()/fbird_prepare_ex() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fbird_prepare() and fbird_prepare_ex() were the last functions returning raw resource handles instead of Firebird\* objects. The Firebird\Statement class existed as a skeleton since v8.0.0 but was never returned. Changes: - Added fbird_setup_statement_object() helper in fbird_classes.c (same pattern as fbird_setup_resultset_object()) - Added fbird_statement_get_resource() for dual-accept bridge - Added Firebird\Statement branch to FBIRD_VALIDATE_QUERY_EX macro - Added Statement instanceof check to fbird_execute() type validation - Wrapped RETVAL_RES in both fbird_prepare() and fbird_prepare_ex() - Updated stubs: return type changed from mixed to Firebird\Statement|false - OOP Connection::prepare() now calls _php_fbird_prepare() directly instead of through call_user_function (avoids segfault) - OOP Statement::execute() now calls _php_fbird_exec() directly instead of through call_user_function Also fixes: - fbird_query_prepare.c: guard autocommit commit to only fire for the DEFAULT transaction (first tr_list node), not temp transactions from fbird_execute_auto() — prevents use-after-free SIGSEGV - fbird_classes.c: Connection::prepare() and Statement::execute() refactored to call internal C functions directly Tests: - Added tests/issue297_statement_return_type.phpt - Updated tests using is_resource() to use instanceof checks - Updated resource_type_validation_001 TypeError message Full test suite: 275/275 PASS (0 failures, 9 skipped). BREAKING CHANGE: fbird_prepare() and fbird_prepare_ex() now return Firebird\Statement objects instead of resources. Code using is_resource() checks must update to instanceof \Firebird\Statement. The dual-accept bridge accepts both types in all consuming functions. --- fbird_classes.c | 97 +++++++++++-------- fbird_classes.h | 12 +++ fbird_query_exec.c | 35 +++++-- fbird_query_prepare.c | 20 ++-- php_fbird_includes.h | 22 +++++ stubs/firebird-stubs.php | 8 +- tests/coverage/execute_statement_query.phpt | 4 +- tests/coverage/query_params_tx.phpt | 4 +- tests/execute_safety_001.phpt | 2 +- tests/issue131.phpt | 11 ++- tests/issue297_statement_return_type.phpt | 100 ++++++++++++++++++++ tests/resource_type_validation_001.phpt | 2 +- 12 files changed, 249 insertions(+), 68 deletions(-) create mode 100644 tests/issue297_statement_return_type.phpt diff --git a/fbird_classes.c b/fbird_classes.c index 98f7962a..729a93dc 100644 --- a/fbird_classes.c +++ b/fbird_classes.c @@ -21,6 +21,7 @@ #include "php_fbird_includes.h" #include "firebird_utils.h" #include "fbird_classes.h" +#include "php_fbird_query_prepare.h" #include "php_fbird_connection.h" #include "php_fbird_query_internal.h" @@ -475,36 +476,37 @@ PHP_METHOD(FirebirdStatement, execute) ZEND_PARSE_PARAMETERS_END(); fbird_statement_obj *intern = Z_FBIRD_STATEMENT_P(ZEND_THIS); - if (!intern->query_res) { - zend_throw_exception(fbird_query_exception_ce, "Statement not prepared", 0); + if (!intern->query_res || intern->query_res->type <= 0) { + zend_throw_exception(fbird_query_exception_ce, "Statement not prepared or freed", 0); RETURN_THROWS(); } - /* Call fbird_execute($query_res) */ - zval res_zv, retval; - ZVAL_RES(&res_zv, intern->query_res); - GC_ADDREF(intern->query_res); - fbird_call_fn("fbird_execute", &res_zv, 1, &retval); - zval_ptr_dtor(&res_zv); + /* Issue #297: Call _php_fbird_exec() directly instead of going through + * fbird_execute() via call_user_function. This avoids argument validation + * overhead and segfaults from call_user_function context issues. */ + fbird_query *ib_query = (fbird_query *)intern->query_res->ptr; + if (!ib_query) { + zend_throw_exception(fbird_query_exception_ce, "Statement has no query data", 0); + RETURN_THROWS(); + } - if (Z_TYPE(retval) == IS_FALSE) { - zval_ptr_dtor(&retval); + RETVAL_FALSE; + if (FAILURE == _php_fbird_exec(INTERNAL_FUNCTION_PARAM_PASSTHRU, ib_query, NULL, 0)) { zend_throw_exception(fbird_query_exception_ce, "Failed to execute statement", 0); RETURN_THROWS(); } - /* Return a ResultSet wrapping the same query resource */ - object_init_ex(return_value, fbird_resultset_ce); - fbird_resultset_obj *rs = Z_FBIRD_RESULTSET_P(return_value); - if (Z_TYPE(retval) == IS_RESOURCE) { - rs->query_res = Z_RES(retval); - GC_ADDREF(rs->query_res); - zval_ptr_dtor(&retval); - } else { - /* Non-SELECT: reuse the prepared statement resource for fetch (returns false) */ + /* Issue #296: wrap le_query result in Firebird\ResultSet (same as fbird_execute) */ + if (Z_TYPE_P(return_value) == IS_RESOURCE && + Z_RES_TYPE_P(return_value) == le_query) { + fbird_setup_resultset_object(return_value, Z_RES_P(return_value)); + } else if (Z_TYPE_P(return_value) != IS_RESOURCE) { + /* Non-SELECT (DML): return a ResultSet wrapping the statement resource + * (fetch will return false — standard behavior for DML results) */ + object_init_ex(return_value, fbird_resultset_ce); + fbird_resultset_obj *rs = Z_FBIRD_RESULTSET_P(return_value); rs->query_res = intern->query_res; GC_ADDREF(rs->query_res); - zval_ptr_dtor(&retval); } } @@ -554,27 +556,20 @@ PHP_METHOD(FirebirdConnection, prepare) RETURN_THROWS(); } - /* Call fbird_prepare($conn_res, $tr_res, $sql) */ - zval prep_args[3], retval; - ZVAL_RES(&prep_args[0], conn->conn_res); - GC_ADDREF(conn->conn_res); - ZVAL_RES(&prep_args[1], tr_res); - GC_ADDREF(tr_res); - ZVAL_STRINGL(&prep_args[2], sql, sql_len); - fbird_call_fn("fbird_prepare", prep_args, 3, &retval); - for (int i = 0; i < 3; i++) zval_ptr_dtor(&prep_args[i]); - - if (Z_TYPE(retval) != IS_RESOURCE) { - zval_ptr_dtor(&retval); + /* Issue #297: Call _php_fbird_prepare() directly instead of through + * fbird_prepare()/call_user_function. This avoids the overhead and + * segfault risk of call_user_function in OOP method context. */ + fbird_db_link *link = (fbird_db_link *)conn->conn_res->ptr; + fbird_transaction *trans = (fbird_transaction *)tr_res->ptr; + fbird_query *ib_query = NULL; + + if (FAILURE == _php_fbird_prepare(&ib_query, link, trans, tr_res, sql)) { zend_throw_exception(fbird_query_exception_ce, "Failed to prepare statement", 0); RETURN_THROWS(); } - object_init_ex(return_value, fbird_statement_ce); - fbird_statement_obj *stmt = Z_FBIRD_STATEMENT_P(return_value); - stmt->query_res = Z_RES(retval); - GC_ADDREF(stmt->query_res); - zval_ptr_dtor(&retval); + /* Wrap the le_query resource in a Firebird\Statement object */ + fbird_setup_statement_object(return_value, ib_query->res); } /* ----------------------------------------------------------------------- @@ -1303,6 +1298,17 @@ zend_resource *fbird_resultset_get_resource(zend_object *obj) return intern ? intern->query_res : NULL; } +/* ----------------------------------------------------------------------- + * fbird_statement_get_resource() — extract zend_resource* from a + * Firebird\Statement internal object. Returns NULL if no resource set. + * Used by the dual-accept bridge in fbird_execute(), fbird_free_query(), etc. + * --------------------------------------------------------------------- */ +zend_resource *fbird_statement_get_resource(zend_object *obj) +{ + fbird_statement_obj *intern = fbird_statement_from_obj(obj); + return intern ? intern->query_res : NULL; +} + /* ----------------------------------------------------------------------- * fbird_setup_resultset_object() — glue from procedural fbird_query() / * fbird_execute() to Firebird\ResultSet @@ -1322,6 +1328,23 @@ void fbird_setup_resultset_object(zval *return_value, zend_resource *res) intern->query_res = res; } +/* ----------------------------------------------------------------------- + * fbird_setup_statement_object() — glue from procedural fbird_prepare() / + * fbird_prepare_ex() to Firebird\Statement + * + * Same pattern as fbird_setup_resultset_object(): wraps the le_query + * resource in a typed Firebird\Statement object. The resource stays in + * EG(regular_list); we store a weak reference. + * --------------------------------------------------------------------- */ +void fbird_setup_statement_object(zval *return_value, zend_resource *res) +{ + zval_ptr_dtor(return_value); + object_init_ex(return_value, fbird_statement_ce); + fbird_statement_obj *intern = fbird_statement_from_obj(Z_OBJ_P(return_value)); + GC_ADDREF(res); + intern->query_res = res; +} + /* ----------------------------------------------------------------------- * Registration entry point called from PHP_MINIT_FUNCTION(fbird) * --------------------------------------------------------------------- */ diff --git a/fbird_classes.h b/fbird_classes.h index 2eba302c..8118af9b 100644 --- a/fbird_classes.h +++ b/fbird_classes.h @@ -54,12 +54,24 @@ void fbird_setup_transaction_object(zval *return_value, zend_resource *res); */ zend_resource *fbird_resultset_get_resource(zend_object *obj); +/** + * Extract the zend_resource* from a Firebird\Statement object. + * Returns NULL if obj is not a Firebird\Statement or has no resource. + */ +zend_resource *fbird_statement_get_resource(zend_object *obj); + /** * Wrap a le_query zend_resource* in a Firebird\ResultSet object. * Stores the resource as a weak reference (EG(regular_list) owns it). */ void fbird_setup_resultset_object(zval *return_value, zend_resource *res); +/** + * Wrap a le_query zend_resource* in a Firebird\Statement object. + * Same weak-reference pattern as fbird_setup_resultset_object(). + */ +void fbird_setup_statement_object(zval *return_value, zend_resource *res); + /** * Extract the zend_resource* from a Firebird\Blob object (M3 Phase G bridge). * Returns NULL if no resource is set (OOP-native path). diff --git a/fbird_query_exec.c b/fbird_query_exec.c index 13aa1c2f..65082c6a 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -82,7 +82,7 @@ static fbird_db_link *_php_fbird_link_from_zval(zval *z) return NULL; } -static int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *ib_query, zval *args, int bind_n) +int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *ib_query, zval *args, int bind_n) { int rv = FAILURE; ISC_STATUS isc_result; @@ -1374,6 +1374,13 @@ PHP_FUNCTION(fbird_prepare) efree(args); RETVAL_RES(ib_query->res); Z_TRY_ADDREF_P(return_value); + + /* Issue #297: wrap le_query result in Firebird\Statement object */ + if (Z_TYPE_P(return_value) == IS_RESOURCE && + Z_RES_TYPE_P(return_value) == le_query) { + zend_resource *_res = Z_RES_P(return_value); + fbird_setup_statement_object(return_value, _res); + } } /* {{{ proto resource fbird_prepare_ex(resource $link, string $query [, resource $trans]) @@ -1432,6 +1439,13 @@ PHP_FUNCTION(fbird_prepare_ex) RETVAL_RES(ib_query->res); Z_TRY_ADDREF_P(return_value); + + /* Issue #297: wrap le_query result in Firebird\Statement object */ + if (Z_TYPE_P(return_value) == IS_RESOURCE && + Z_RES_TYPE_P(return_value) == le_query) { + zend_resource *_res = Z_RES_P(return_value); + fbird_setup_statement_object(return_value, _res); + } } /* }}} */ @@ -1451,14 +1465,16 @@ PHP_FUNCTION(fbird_execute) WRONG_PARAM_COUNT; } - /* Validate first argument: query resource OR Firebird\ResultSet object. - * Throw TypeError for wrong types (consistent with other fbird_* functions). */ + /* Validate first argument: query resource OR Firebird\ResultSet/Statement object. + * Throw TypeError for wrong types (consistent with other fbird_* functions). + * Issue #297: also accept Firebird\Statement (returned by fbird_prepare/ex). */ if (Z_TYPE(args[0]) != IS_RESOURCE && !(Z_TYPE(args[0]) == IS_OBJECT && - instanceof_function(Z_OBJCE(args[0]), fbird_resultset_ce))) { + (instanceof_function(Z_OBJCE(args[0]), fbird_resultset_ce) || + instanceof_function(Z_OBJCE(args[0]), fbird_statement_ce)))) { /* Capture type name BEFORE efree(args) to avoid use-after-free */ const char *arg_type = zend_get_type_by_const(Z_TYPE(args[0])); - zend_type_error("fbird_execute(): Argument #1 ($query) must be a Firebird query resource or Firebird\\ResultSet, %s given", arg_type); + zend_type_error("fbird_execute(): Argument #1 ($query) must be a Firebird query resource or Firebird\\ResultSet/Statement, %s given", arg_type); efree(args); RETURN_THROWS(); } @@ -1504,13 +1520,20 @@ void _php_fbird_free_query_impl(INTERNAL_FUNCTION_PARAMETERS, int as_result) return; } - /* M3 Phase G: Accept Firebird\ResultSet objects */ + /* M3 Phase G: Accept Firebird\ResultSet and Firebird\Statement objects */ if (Z_TYPE_P(query_arg) == IS_OBJECT && instanceof_function(Z_OBJCE_P(query_arg), fbird_resultset_ce)) { res = fbird_resultset_get_resource(Z_OBJ_P(query_arg)); if (!res) { RETURN_FALSE; } + } else if (Z_TYPE_P(query_arg) == IS_OBJECT && + instanceof_function(Z_OBJCE_P(query_arg), fbird_statement_ce)) { + /* Issue #297: Firebird\Statement returned by fbird_prepare()/fbird_prepare_ex() */ + res = fbird_statement_get_resource(Z_OBJ_P(query_arg)); + if (!res) { + RETURN_FALSE; + } } else { if (Z_TYPE_P(query_arg) != IS_RESOURCE) { RETURN_FALSE; diff --git a/fbird_query_prepare.c b/fbird_query_prepare.c index febf1349..6c882ac7 100755 --- a/fbird_query_prepare.c +++ b/fbird_query_prepare.c @@ -221,19 +221,19 @@ void php_fbird_free_query_rsrc(zend_resource *rsrc) /* Issue #294: Commit the default transaction so the next autocommit * query starts a fresh transaction with a current snapshot. * - * Skip for persistent connections (is_persistent): cleanup_db() may - * drop the DB during shutdown, making the attachment dead. The - * default tx is cleaned up by _php_fbird_commit_link during - * MSHUTDOWN (with #295 getMaster() guard). Non-persistent - * connections (doctrine-firebird-driver) get the full fix. + * Only fire for the DEFAULT transaction (first tr_list node). + * fbird_execute_auto() creates a temp transaction not in tr_list — + * freeing it here would cause use-after-free when execute_auto + * later calls fbt_rollback on the same pointer. * - * fbt_free is now called — the #295 fix to rollbackNoThrow() - * (getMaster() guard) makes it safe: rollbackNoThrow() returns - * early when transaction_ is null (already committed), so only - * the C++ delete runs, cleaning up the wrapper object. + * Skip for persistent connections: cleanup_db() may drop the DB + * during shutdown. The default tx is cleaned up by + * _php_fbird_commit_link during MSHUTDOWN (with #295 guard). */ + bool is_default_tx = (ib_query->link && ib_query->link->tr_list && + ib_query->link->tr_list->trans == ib_query->trans); bool is_persistent = (ib_query->link && ib_query->link->is_persistent); - if (!is_persistent) { + if (is_default_tx && !is_persistent) { fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); fbt_free(ib_query->trans->fbt_transaction); ib_query->trans->fbt_transaction = NULL; diff --git a/php_fbird_includes.h b/php_fbird_includes.h index b0eeb77e..24efd0cc 100755 --- a/php_fbird_includes.h +++ b/php_fbird_includes.h @@ -474,6 +474,9 @@ const char *_fbird_res_type_name(int type); /* Validate query/result resource (le_query). * M3 Phase G: Also accepts Firebird\ResultSet objects (weak-ref to same resource). */ +/* Issue #297: Exported for OOP Statement::execute() to call directly */ +int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *ib_query, zval *args, int bind_n); + #define FBIRD_VALIDATE_QUERY_EX(zv, argnum, var) do { \ /* M3 object path: Firebird\ResultSet accepted alongside resources */ \ if (Z_TYPE_P(zv) == IS_OBJECT && \ @@ -494,6 +497,25 @@ const char *_fbird_res_type_name(int type); } \ break; \ } \ + /* Issue #297: Firebird\Statement (from fbird_prepare/ex) accepted */ \ + if (Z_TYPE_P(zv) == IS_OBJECT && \ + instanceof_function(Z_OBJCE_P(zv), fbird_statement_ce)) { \ + zend_resource *_qres = fbird_statement_get_resource(Z_OBJ_P(zv)); \ + if (!_qres) { RETURN_FALSE; } \ + var = (fbird_query *)_qres->ptr; \ + if (!var) { \ + if (IBG(exception_mode) == FBIRD_EXCEPTION_MODE_THROW) { \ + zend_argument_type_error(argnum, \ + "must be a valid (non-freed) Firebird query resource or Firebird\\Statement"); \ + RETURN_THROWS(); \ + } \ + php_error_docref(NULL, E_WARNING, \ + "Argument #%d must be a valid (non-freed) Firebird query resource or Firebird\\Statement", \ + argnum); \ + RETURN_FALSE; \ + } \ + break; \ + } \ /* Must be IS_RESOURCE - anything else (object, array, etc.) is a type error */ \ if (Z_TYPE_P(zv) != IS_RESOURCE) { \ if (IBG(exception_mode) == FBIRD_EXCEPTION_MODE_THROW) { \ diff --git a/stubs/firebird-stubs.php b/stubs/firebird-stubs.php index 2b8d6028..6a462791 100644 --- a/stubs/firebird-stubs.php +++ b/stubs/firebird-stubs.php @@ -421,14 +421,14 @@ function fbird_query(mixed $link_or_query, mixed ...$args): \Firebird\ResultSet| * @param resource|string|null $link_or_trans_or_query First argument * @param resource|string|null $link_or_trans_or_query_2 Second argument * @param string|null $query Query string - * @return resource|false Prepared statement resource + * @return \Firebird\Statement|false Statement object (v11.1.0+, was resource before) * @since 7.0.0 */ function fbird_prepare( mixed $link_or_trans_or_query, mixed $link_or_trans_or_query_2 = null, ?string $query = null -): mixed {} +): \Firebird\Statement|false {} /** * Prepare a SQL statement with a fixed, non-shifting signature. @@ -440,14 +440,14 @@ function fbird_prepare( * @param resource $link_identifier Connection resource * @param string $query SQL query string * @param resource|null $trans_handle Transaction resource (optional) - * @return resource|false Prepared statement resource or false on failure + * @return \Firebird\Statement|false Statement object (v11.1.0+, was resource before) * @since 9.0.0 */ function fbird_prepare_ex( mixed $link_identifier, string $query, mixed $trans_handle = null -): mixed {} +): \Firebird\Statement|false {} /** * Execute a prepared statement. diff --git a/tests/coverage/execute_statement_query.phpt b/tests/coverage/execute_statement_query.phpt index a019bc44..8c674ddb 100644 --- a/tests/coverage/execute_statement_query.phpt +++ b/tests/coverage/execute_statement_query.phpt @@ -42,7 +42,7 @@ var_dump($r !== false); // Test 4: fbird_execute_query — SELECT (returns result resource) echo "Test 4: fbird_execute_query SELECT\n"; $res = fbird_execute_query($tr, "SELECT ID, VAL FROM EXEC_STMT_COV ORDER BY ID"); -var_dump($res !== false && is_resource($res)); +var_dump($res !== false && $res instanceof \Firebird\ResultSet); if ($res) { while ($row = fbird_fetch_assoc($res)) { // consume rows @@ -53,7 +53,7 @@ if ($res) { // Test 5: fbird_execute_query — SELECT with params echo "Test 5: fbird_execute_query SELECT with params\n"; $res = fbird_execute_query($tr, "SELECT VAL FROM EXEC_STMT_COV WHERE ID = ?", [1]); -var_dump($res !== false && is_resource($res)); +var_dump($res !== false && $res instanceof \Firebird\ResultSet); if ($res) { $row = fbird_fetch_row($res); var_dump($row[0] === 'world'); diff --git a/tests/coverage/query_params_tx.phpt b/tests/coverage/query_params_tx.phpt index 0b808bd5..e794b7ae 100644 --- a/tests/coverage/query_params_tx.phpt +++ b/tests/coverage/query_params_tx.phpt @@ -42,7 +42,7 @@ $rs = fbird_query_params_tx( "SELECT ITEST, CTEST FROM qptx_test WHERE ITEST = ?", [9901] ); -var_dump(is_resource($rs)); +var_dump($rs instanceof \Firebird\ResultSet); $row = fbird_fetch_assoc($rs); var_dump($row !== false); @@ -56,7 +56,7 @@ $rs2 = fbird_query_params_tx( "SELECT ITEST FROM qptx_test WHERE ITEST = ?", [99999] ); -var_dump(is_resource($rs2)); +var_dump($rs2 instanceof \Firebird\ResultSet); var_dump(fbird_fetch_assoc($rs2) === false); fbird_free_result($rs2); diff --git a/tests/execute_safety_001.phpt b/tests/execute_safety_001.phpt index b5d60669..a8d2c0aa 100755 --- a/tests/execute_safety_001.phpt +++ b/tests/execute_safety_001.phpt @@ -46,7 +46,7 @@ assert_exception(function() use ($trans) { // 4. Correct Usage of fbird_execute_query (SELECT) echo "4. SELECT via execute_query...\n"; $res = fbird_execute_query($trans, "select * from test_exec_safety"); -var_dump(is_resource($res)); // bool(true) +var_dump($res instanceof \Firebird\ResultSet); // bool(true) $row = fbird_fetch_row($res); var_dump($row[0]); // int(1) fbird_free_result($res); diff --git a/tests/issue131.phpt b/tests/issue131.phpt index dae6fb92..be8a1b6c 100644 --- a/tests/issue131.phpt +++ b/tests/issue131.phpt @@ -15,15 +15,16 @@ echo "Executing query in transaction...\n"; $q = fbird_query($tr, "SELECT * FROM RDB\$DATABASE"); echo "Checking result before commit_ret...\n"; -if (is_resource($q)) echo "Result is a resource\n"; +/* Issue #296: fbird_query() now returns Firebird\ResultSet object, not resource */ +if ($q instanceof \Firebird\ResultSet) echo "Result is a Firebird\ResultSet object\n"; echo "Calling fbird_commit_ret()...\n"; fbird_commit_ret($tr); echo "Checking result AFTER commit_ret...\n"; // Result resource should remain valid according to Firebird API if we use commit_ret -if (is_resource($q)) { - echo "OK: Result resource still exists after commit_ret\n"; +if ($q instanceof \Firebird\ResultSet) { + echo "OK: Result object still exists after commit_ret\n"; $row = fbird_fetch_assoc($q); if ($row) { echo "OK: Fetched row after commit_ret\n"; @@ -40,8 +41,8 @@ fbird_close($conn); Starting transaction... Executing query in transaction... Checking result before commit_ret... -Result is a resource +Result is a Firebird\ResultSet object Calling fbird_commit_ret()... Checking result AFTER commit_ret... -OK: Result resource still exists after commit_ret +OK: Result object still exists after commit_ret OK: Fetched row after commit_ret diff --git a/tests/issue297_statement_return_type.phpt b/tests/issue297_statement_return_type.phpt new file mode 100644 index 00000000..1c3bb37c --- /dev/null +++ b/tests/issue297_statement_return_type.phpt @@ -0,0 +1,100 @@ +--TEST-- +fbird_prepare()/fbird_prepare_ex() return Firebird\Statement object (Issue #297) +--EXTENSIONS-- +firebird +--SKIPIF-- + +--FILE-- + +--EXPECT-- +fbird_prepare type: object +fbird_prepare instanceof Statement: yes +fbird_execute on Statement type: object +fbird_execute instanceof ResultSet: yes +fbird_prepare_ex type: object +fbird_prepare_ex instanceof Statement: yes +fbird_prepare_ex result value: 2 +re-execute result value: 1 +fbird_prepare with tx type: object +fbird_prepare with tx instanceof Statement: yes +fbird_prepare with tx result value: 3 +Done diff --git a/tests/resource_type_validation_001.phpt b/tests/resource_type_validation_001.phpt index 8338bf2a..06a88810 100644 --- a/tests/resource_type_validation_001.phpt +++ b/tests/resource_type_validation_001.phpt @@ -114,7 +114,7 @@ echo "\n=== All tests completed ===\n"; ?> --EXPECTF-- === Test 1: fbird_execute() with connection instead of query === -TypeError caught: fbird_execute(): Argument #1 ($query) must be a Firebird query resource or Firebird\ResultSet, object given +TypeError caught: fbird_execute(): Argument #1 ($query) must be a Firebird query resource or Firebird\ResultSet/Statement, object given === Test 2: fbird_fetch_assoc() with connection instead of query === TypeError caught: fbird_fetch_assoc(): Argument #1 ($result) must be a Firebird query/result resource, object given From 3d46bfca0f248400de2423ceeb751b710672bc66 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 19:35:51 +0200 Subject: [PATCH 7/9] chore: bump version to 11.1.0 + update CHANGELOG Minor release (v11.1.0) completing the M3 resource-to-object migration. All stubs updated to @version 11.1.0. VERSION.txt updated. CHANGELOG updated with [11.1.0] section covering all 4 issues (#294-#297). Full test suite: 275/275 PASS (0 failures, 9 skipped). --- CHANGELOG.md | 73 +++++++++++++++++++++++--------------- VERSION.txt | 2 +- stubs/firebird-classes.php | 2 +- stubs/firebird-stubs.php | 2 +- stubs/pdo-fbird-stubs.php | 2 +- 5 files changed, 49 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c808f9f..f87f81d6 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [11.0.2] - 2026-07-02 +## [11.1.0] - 2026-07-02 + +### Summary +Minor release completing the M3 resource-to-object migration. All handle-returning +`fbird_*` functions now return typed `Firebird\*` objects. No raw resources are +returned to userland. The dual-accept bridge accepts both legacy resources and +new objects in all consuming functions — no consumer-side changes required. ### Fixed - **[Issue #294]** `fbird_query($conn, $sql)` autocommit mode didn't see data committed @@ -26,39 +32,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `master_` with `getMaster()` in all three methods. Added `in_mshutdown` guard in `fbird_transaction_free` as belt-and-suspenders. (`src/cpp/fb_transaction.hpp`, `fbird_classes.c`) +- **[Issue #296]** `fbird_query()` stub declared `\Firebird\ResultSet` return type but + C code returned raw `resource`. Same issue affected `fbird_execute_query()` and + `fbird_query_params_tx()`. Fix: added `fbird_setup_resultset_object()` wrapping block + to all three functions (same pattern as `fbird_execute()`). (`fbird_query_exec.c`) + +### Added +- **[Issue #297]** `fbird_prepare()` and `fbird_prepare_ex()` now return `Firebird\Statement` + objects instead of raw `resource` handles. Completes the M3 resource-to-object migration + — all handle-returning `fbird_*` functions now return typed objects. Added + `fbird_setup_statement_object()` and `fbird_statement_get_resource()` helpers. + `Firebird\Statement` branch added to `FBIRD_VALIDATE_QUERY_EX` macro and `fbird_execute()` + type validation. OOP `Connection::prepare()` and `Statement::execute()` refactored to + call internal C functions directly (avoids `call_user_function` segfault). + (`fbird_classes.c`, `fbird_classes.h`, `fbird_query_exec.c`, `php_fbird_includes.h`) +- `is_persistent` field added to `fbird_db_link` struct to distinguish persistent from + non-persistent connections in autocommit logic. ### Tests -- Added `tests/issue294_autocommit_visibility.phpt` — regression test verifying - autocommit query sees committed data after explicit-tx commit. -- Added `tests/issue295_mshutdown_pconnect_sigsegv.phpt` — regression test for - MSHUTDOWN cleanup with active default transaction on persistent connection. -- Updated `tests/005.phpt` — use explicit transaction for rollback test (autocommit - DML is now committed immediately). Added `@fbird_commit()` suppression for - already-committed default tx in `tests/fbird_query_stmt_release_001.phpt` and - `tests/issue35_001.phpt`. -- Updated `tests/fbird_rollback_001.phpt`, `tests/fbird_trans_008.phpt`, - `tests/fbird_trans_009.phpt`, `tests/004.phpt`, `tests/fbird_batch_blob_001.phpt` - to use explicit transactions where rollback/savepoints are needed (autocommit - DML is now committed immediately and cannot be rolled back). +- Added `tests/issue294_autocommit_visibility.phpt` — autocommit sees committed data. +- Added `tests/issue295_mshutdown_pconnect_sigsegv.phpt` — MSHUTDOWN cleanup with default tx. +- Added `tests/issue296_resultset_return_type.phpt` — all SELECT-returning functions return + `Firebird\ResultSet` objects. +- Added `tests/issue297_statement_return_type.phpt` — `fbird_prepare/ex` return + `Firebird\Statement` objects, dual-accept bridge works with `fbird_execute()`. +- Updated 7 existing tests to use `instanceof` checks instead of `is_resource()`. ### Additional Fixes - `fbird_commit()` and `fbird_rollback()` on the default link are now silent no-ops - (return `true`) when the default transaction was already committed by autocommit, - instead of warning "invalid transaction handle". Explicit transactions still warn. - (`fbird_transaction.c`) + (return `true`) when the default transaction was already committed by autocommit. - `fbird_execute()` on prepared statements now restarts the default transaction if - it was committed by a prior autocommit DML call. Previously, `fbird_execute()` - would fail with "Legacy API execution not supported" if a `fbird_query()` DML - call committed the default transaction between prepare and execute. - (`fbird_query_exec.c`) - -### Known Limitations -- **Transaction C++ object leak**: `fbt_free` is not called in the autocommit path - (`php_fbird_free_query_rsrc`) because calling it caused SIGSEGV (StatusWrapper - destructor). The `Transaction` C++ object is cleaned up by `_php_fbird_commit_link` - during connection close. Small leak (one object per autocommit SELECT on non-persistent - connections). Fix in follow-up by adding `fbt_dispose()` that clears `last_status_` - before `delete`. + it was committed by a prior autocommit DML call. +- Autocommit commit in `php_fbird_free_query_rsrc` guarded to only fire for the + default transaction (first `tr_list` node), preventing use-after-free in + `fbird_execute_auto()` temp transactions. + +### Breaking Changes (v11.1) +- Return types of `fbird_prepare()` and `fbird_prepare_ex()` changed from `resource` to + `Firebird\Statement`. Code using `is_resource()` checks will need updating to + `$x instanceof \Firebird\Statement`. +- `fbird_query()`, `fbird_execute_query()`, `fbird_query_params_tx()` now return + `Firebird\ResultSet` objects for SELECT (was `resource`). The stubs already declared + this type since v11.0.0 — this fix makes the C code match the stubs. +- `fbird_commit()`/`fbird_rollback()` on already-committed default transaction now + silently returns `true` (was: warning + `false`). ## [11.0.1] - 2026-04-20 diff --git a/VERSION.txt b/VERSION.txt index a1ea332d..68d8f15e 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -11.0.2 +11.1.0 diff --git a/stubs/firebird-classes.php b/stubs/firebird-classes.php index bd0708d1..b016ebfd 100644 --- a/stubs/firebird-classes.php +++ b/stubs/firebird-classes.php @@ -7,7 +7,7 @@ * C extension. These are used by static analysis tools and IDEs. * * @package php-firebird-stubs - * @version 11.0.2 + * @version 11.1.0 * @author satware AG * @copyright 2025 satware AG * @license PHP-3.01 https://www.php.net/license/3_01.txt diff --git a/stubs/firebird-stubs.php b/stubs/firebird-stubs.php index 6a462791..dc308ee4 100644 --- a/stubs/firebird-stubs.php +++ b/stubs/firebird-stubs.php @@ -10,7 +10,7 @@ * This stub file does not contain any implementation. * * @package php-firebird-stubs - * @version 11.0.2 + * @version 11.1.0 * @author satware AG * @copyright 2025-2026 satware AG * @license PHP-3.01 https://www.php.net/license/3_01.txt diff --git a/stubs/pdo-fbird-stubs.php b/stubs/pdo-fbird-stubs.php index b7119863..88d9c366 100644 --- a/stubs/pdo-fbird-stubs.php +++ b/stubs/pdo-fbird-stubs.php @@ -15,7 +15,7 @@ * 'SYSDBA', 'masterkey'); * * @package php-firebird-stubs - * @version 11.0.2 + * @version 11.1.0 * @author satware AG * @copyright 2025-2026 satware AG * @license PHP-3.01 https://www.php.net/license/3_01.txt From a685fbc20fa946c83509bc8d9283d0e0707763c6 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 19:59:07 +0200 Subject: [PATCH 8/9] fix(review): check fbt_commit() return value in autocommit paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review found that fbt_commit() errors were silently swallowed in both autocommit sites (fbird_query_exec.c DML path, fbird_query_prepare.c SELECT-result-free path). If commit failed (e.g., deferred FK constraint violation, connection lost), the error was discarded and fbt_free() rolled back the data — the user's DML appeared to succeed but wasn't committed. Fix: check the return value and call _php_fbird_error() on failure, matching the established pattern in _php_fbird_commit_link. --- fbird_query_exec.c | 13 ++++++++----- fbird_query_prepare.c | 7 ++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/fbird_query_exec.c b/fbird_query_exec.c index 65082c6a..1a138278 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -1226,13 +1226,16 @@ PHP_FUNCTION(fbird_query) bool is_persistent = (link && link->is_persistent); if (!trans_res && trans && trans->fbt_transaction && Z_TYPE_P(return_value) != IS_RESOURCE && !is_persistent) { - /* Issue #294: Commit + free the default transaction for true autocommit. - * fbt_free calls rollbackNoThrow() (safe — transaction_ is null after - * commit, so it returns early) then deletes the C++ Transaction object, - * preventing the wrapper leak identified in code review. */ - fbt_commit(trans->fbt_transaction, IB_STATUS); + /* Issue #294: Commit + free the default transaction for true autocommit. + * fbt_free calls rollbackNoThrow() (safe — transaction_ is null after + * commit, so it returns early) then deletes the C++ Transaction object, + * preventing the wrapper leak identified in code review. */ + int _ac_res = fbt_commit(trans->fbt_transaction, IB_STATUS); fbt_free(trans->fbt_transaction); trans->fbt_transaction = NULL; + if (_ac_res && !IBG(in_mshutdown)) { + _php_fbird_error(); + } } } diff --git a/fbird_query_prepare.c b/fbird_query_prepare.c index 6c882ac7..cb002b28 100755 --- a/fbird_query_prepare.c +++ b/fbird_query_prepare.c @@ -234,9 +234,14 @@ void php_fbird_free_query_rsrc(zend_resource *rsrc) ib_query->link->tr_list->trans == ib_query->trans); bool is_persistent = (ib_query->link && ib_query->link->is_persistent); if (is_default_tx && !is_persistent) { - fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); + int _ac_res = fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); fbt_free(ib_query->trans->fbt_transaction); ib_query->trans->fbt_transaction = NULL; + /* Report commit errors (e.g., deferred constraint violations). + * IBG(in_mshutdown) already guarded by outer condition. */ + if (_ac_res) { + _php_fbird_error(); + } } } From 88c4f236e50381cbfb1d2d5b267961585ada1d09 Mon Sep 17 00:00:00 2001 From: Michael Wegener Date: Thu, 2 Jul 2026 20:11:53 +0200 Subject: [PATCH 9/9] =?UTF-8?q?fix(review):=20silent=20autocommit=20commit?= =?UTF-8?q?=20=E2=80=94=20cursor-lock=20failures=20are=20expected?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /review suggested checking fbt_commit() return value and reporting errors. However, the autocommit commit can legitimately fail when the default transaction has open cursors from a prior SELECT (Firebird error: 'object TABLE ... is in use'). Reporting these as errors would be noise — the transaction stays valid and is committed at connection close or explicit fbird_commit(). Reverted to silent commit (no error reporting). Added jane: intent comment documenting the rationale. Full test suite: 275/275 PASS (0 failures, 9 skipped). --- fbird_query_exec.c | 17 +++++++++-------- fbird_query_prepare.c | 8 ++------ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/fbird_query_exec.c b/fbird_query_exec.c index 1a138278..6e77df17 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -1226,16 +1226,17 @@ PHP_FUNCTION(fbird_query) bool is_persistent = (link && link->is_persistent); if (!trans_res && trans && trans->fbt_transaction && Z_TYPE_P(return_value) != IS_RESOURCE && !is_persistent) { - /* Issue #294: Commit + free the default transaction for true autocommit. - * fbt_free calls rollbackNoThrow() (safe — transaction_ is null after - * commit, so it returns early) then deletes the C++ Transaction object, - * preventing the wrapper leak identified in code review. */ - int _ac_res = fbt_commit(trans->fbt_transaction, IB_STATUS); + /* Issue #294: Commit + free the default transaction for true autocommit. + * fbt_free calls rollbackNoThrow() (safe — transaction_ is null after + * commit, so it returns early) then deletes the C++ Transaction object. + * If commit fails (e.g., open cursors from a prior SELECT on the same + * default tx), silently continue — the transaction stays valid and + * will be committed at connection close or explicit fbird_commit(). + * jane: silent on failure — autocommit is an optimization, not a + * user-initiated commit; reporting cursor-lock errors would be noise. */ + fbt_commit(trans->fbt_transaction, IB_STATUS); fbt_free(trans->fbt_transaction); trans->fbt_transaction = NULL; - if (_ac_res && !IBG(in_mshutdown)) { - _php_fbird_error(); - } } } diff --git a/fbird_query_prepare.c b/fbird_query_prepare.c index cb002b28..008b62a7 100755 --- a/fbird_query_prepare.c +++ b/fbird_query_prepare.c @@ -234,14 +234,10 @@ void php_fbird_free_query_rsrc(zend_resource *rsrc) ib_query->link->tr_list->trans == ib_query->trans); bool is_persistent = (ib_query->link && ib_query->link->is_persistent); if (is_default_tx && !is_persistent) { - int _ac_res = fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); + /* jane: silent on failure — see fbird_query_exec.c for rationale */ + fbt_commit(ib_query->trans->fbt_transaction, IB_STATUS); fbt_free(ib_query->trans->fbt_transaction); ib_query->trans->fbt_transaction = NULL; - /* Report commit errors (e.g., deferred constraint violations). - * IBG(in_mshutdown) already guarded by outer condition. */ - if (_ac_res) { - _php_fbird_error(); - } } }