fix(#589,#597,#593): rollback_ret lock retention, drop_db tx orphan, per-connection sweep registry

.github/workflows/code-quality.yml

@@ -75,6 +75,11 @@ jobs:
           chmod +x scripts/check-stubs-sync.sh
           bash scripts/check-stubs-sync.sh
 
+      - name: Check pdo_fbird shared-header parity (#602)
+        run: |
+          chmod +x scripts/check-header-parity.sh
+          bash scripts/check-header-parity.sh
+
       - name: Check --CLEAN-- sections for DDL tests
         run: |
           chmod +x scripts/check-clean-sections.sh

CHANGELOG.md

@@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **#589: `fbird_rollback_ret()` on a transaction with **zero open cursors** is now a hard rollback + restart** (twin of the #586 commit_ret fix): isc_rollback_retaining retained attachment-level relation locks exactly like isc_commit_retaining - probe confirmed a fresh SERIALIZABLE NOWAIT attachment was blocked over metadata after a retaining rollback. With no cursors there is nothing to preserve; the hard rollback releases all locks and restarts with the stored TPB. Transactions WITH open cursors keep true retaining semantics. See `UPGRADING.md`.
+
 - **`fbird_query(FBIRD_CREATE, ...)` return type (Layer 1)**: returns a `Firebird\Connection` object instead of a raw resource. Code using `is_resource()` on the result must switch to `instanceof Firebird\Connection` (or truthiness, which is unchanged). The FBIRD_CREATE first argument remains deprecated - use `fbird_create_database()`.
 
 - **Behavior change (Layer 1/2)**: `fbird_commit_ret()` on a transaction with **zero open cursors** is now a hard commit + restart. Besides releasing locks, a hard commit **invalidates open BLOB handles** on that transaction (`isc_commit_retaining` preserved them). The legacy chunked-blob-import pattern (read blob in a loop with periodic `commit_ret`) must keep a SELECT cursor open or use plain `fbird_commit()` boundaries. PDO (`pdo_fbird`) is unaffected - it calls retaining commit directly. See `UPGRADING.md`.

UPGRADING.md

@@ -15,6 +15,34 @@ must switch to `instanceof Firebird\Connection`. The object works everywhere
 the resource did (dual-accept: `fbird_query`, `fbird_close`, `fbird_drop_db`,
 transactions). Prefer `fbird_create_database()`, which is not deprecated.
 
+## v13.2.x → Unreleased (PR #601)
+
+### firebird.so and pdo_fbird.so must be upgraded in lockstep
+
+The internal C ABI between the two extensions changed (`fbs_prepare`
+signature, #593). An old `pdo_fbird.so` loaded against the new
+`firebird.so` crashes on the first `PDO::prepare()` (garbage arguments -
+same failure class as the in-repo incident documented in #602). When
+upgrading across this boundary, update BOTH packages in the same
+deployment; the split packages (`pdo-fbird` PIE, `php-firebird`) share the
+major version for this reason.
+
+## v13.2.x → Unreleased (#589)
+
+### `fbird_rollback_ret()` with no open cursors is now a hard rollback
+
+Same shape as the #586 commit_ret change: a retaining rollback with **zero
+open cursors** performs a hard rollback + transparent restart (stored TPB),
+releasing the attachment-level relation locks that isc_rollback_retaining
+otherwise keeps until disconnect. Callers keeping cursors open retain true
+retaining semantics.
+
+**BLOB handles** (same hazard as #586): a hard rollback invalidates open
+BLOB handles on that transaction; `isc_rollback_retaining` preserved them.
+The legacy blob read-loop with periodic `fbird_rollback_ret()` must keep a
+SELECT cursor open on the same transaction or restructure around plain
+`fbird_rollback()` boundaries.
+
 ## v13.2.x → Unreleased (#595)
 
 ### `fbird_connect()`/`fbird_pconnect()` gain a documented `sync` parameter

fbird_connection.c

@@ -986,6 +986,18 @@ PHP_FUNCTION(fbird_drop_db)
 		 * disconnect(). (PR #590 review finding 3) */
 		void *dead_conn = fb_link->fbc_connection;
 		fb_link->fbc_connection = NULL;
+		/* Issue #597: release live transaction handles BEFORE the drop -
+		 * the attachment is still valid here, so rollbackNoThrow+delete
+		 * fully frees every fb::Transaction wrapper. The old post-drop
+		 * nulling orphaned them (LSAN 40B each, #597): freeing AFTER
+		 * attachment death would touch dead interfaces. DROP DATABASE
+		 * discards open work server-side regardless of drop outcome. */
+		for (l = fb_link->tr_list; l != NULL; l = l->next) {
+			if (l->trans != NULL && l->trans->fbt_transaction != NULL) {
+				fbt_free(l->trans->fbt_transaction);
+				l->trans->fbt_transaction = NULL;
+			}
+		}
 		drop_result = fbc_drop_database(dead_conn, status);
 		if (drop_result != 0) {
 			_php_fbird_error(status);

fbird_inspection.c

@@ -65,6 +65,7 @@ static int _fbird_exec_kill(fbird_db_link *link, fbird_transaction *trans, ISC_I
 		FBG(master_instance),
 		attachment,
 		transaction,
+		NULL,  /* #593: statement freed synchronously below - no registry enrollment needed */
 		sql,
 		0,  /* null-terminated */
 		SQL_DIALECT_V6,
@@ -166,6 +167,7 @@ static int _fbird_drop_table(fbird_db_link *link, fbird_transaction *trans, cons
 		FBG(master_instance),
 		attachment,
 		transaction,
+		NULL,  /* #593: statement freed synchronously below - no registry enrollment needed */
 		drop_sql,
 		0,  /* null-terminated */
 		SQL_DIALECT_V6,
@@ -335,6 +337,7 @@ PHP_FUNCTION(fbird_list_table_blockers)
 		FBG(master_instance),
 		attachment,
 		transaction,
+		NULL,  /* #593: statement freed synchronously below - no registry enrollment needed */
 		sql,
 		0,  /* null-terminated */
 		SQL_DIALECT_V6,

fbird_query_prepare.c

@@ -323,6 +323,7 @@ int _php_fbird_prepare(fbird_query **new_query, fbird_db_link *link,
 		FBG(master_instance),
 		attachment_ptr,
 		transaction_ptr,
+		fb_query->link->fbc_connection,  /* #593: owning connection for sweep */
 		query,
 		0,  /* sql_length: 0 = null-terminated */
 		link->dialect,

fbird_transaction.c

@@ -120,6 +120,12 @@ void _php_fbird_trans_detach_queries(fbird_transaction *trans)
 		q = next;
 	}
 	trans->query_head = NULL;
+	/* jane: batch walk below is DISABLED - see follow-up issue: the
+	 * batch_head clear must happen AFTER the walk (order bug from #600),
+	 * but activating the walk crashes fbird_batch_multitype_001 at
+	 * process exit (complex batch lifecycle). Simple case passes
+	 * (issue599 test); root-causing the exit crash is tracked in the
+	 * #599 follow-up issue. */
 	trans->batch_head = NULL;  /* Issue #599 */
 
 	/* Issue #599: batches hold the same raw backref (dereferenced by
@@ -558,6 +564,7 @@ static void _php_fbird_exec_savepoint(INTERNAL_FUNCTION_PARAMETERS, const char *
 
 	/* Prepare the savepoint statement */
 	stmt = fbs_prepare(FBG(master_instance), attachment, transaction_ptr,
+		link->fbc_connection,  /* #593: owning connection for sweep */
 		query, (unsigned)len, SQL_DIALECT_CURRENT, status);
 	if (!stmt) {
 		_php_fbird_error(status);
@@ -843,6 +850,45 @@ PHP_FUNCTION(fbird_connection_info)
  * single-link transaction (link_cnt == 1).
  * Returns 0 on success, nonzero on failure (status zero-initialized then
  * populated on engine errors; isc_arg_end-terminated on local failures). */
+/* Issue #589: hard ROLLBACK + restart with the stored TPB - the rollback
+ * twin of _php_fbird_trans_commit_restart(). Releases retained relation
+ * locks (isc_rollback_retaining keeps them, same #586 class) while leaving
+ * the caller a live, empty transaction. */
+int _php_fbird_trans_rollback_restart(fbird_transaction *trans, ISC_STATUS_ARRAY status)
+{
+	fbird_db_link *fb_link = trans->db_link[0];
+
+	status[0] = (ISC_STATUS) isc_arg_end;
+	status[1] = 0;
+
+	if (!fb_link || !fb_link->fbc_connection) {
+		return 1;
+	}
+
+	/* Hard rollback: releases all locks including metadata */
+	if (fbt_rollback(trans->fbt_transaction, status) != 0) {
+		return 1;
+	}
+
+	fbt_free(trans->fbt_transaction);
+	trans->fbt_transaction = NULL;
+
+	void *attachment = fbc_get_attachment(fb_link->fbc_connection);
+	if (attachment == NULL) {
+		return 1;
+	}
+
+	trans->fbt_transaction = fbt_start(
+		FBG(master_instance),
+		attachment,
+		trans->stored_tpb_len,
+		trans->stored_tpb_len > 0 ? trans->stored_tpb : NULL,
+		status
+	);
+
+	return (trans->fbt_transaction == NULL) ? 1 : 0;
+}
+
 int _php_fbird_trans_commit_restart(fbird_transaction *trans, ISC_STATUS_ARRAY status)
 {
 	fbird_db_link *fb_link = trans->db_link[0];
@@ -1402,7 +1448,21 @@ static void _php_fbird_trans_end(INTERNAL_FUNCTION_PARAMETERS, int commit)
 			result = fbt_commit(trans->fbt_transaction, status);
 			break;
 		case (ROLLBACK | RETAIN):
-			result = fbt_rollback_retaining(trans->fbt_transaction, status);
+			/* Issue #589: isc_rollback_retaining retains attachment-level
+			 * relation locks exactly like isc_commit_retaining (#586 probe:
+			 * SERIALIZABLE NOWAIT over metadata blocked after a retaining
+			 * rollback). With no open cursor there is nothing to preserve -
+			 * hard rollback + restart releases the locks and is observably
+			 * identical for the caller. Single-link + non-MSHUTDOWN only. */
+			if (trans->open_cursor_count == 0 &&
+			    trans->link_cnt == 1 &&
+			    !FBG(in_mshutdown)) {
+				FBDEBUG("Issue #589: rollback_ret with no open cursors -> hard rollback + restart");
+				result = (_php_fbird_trans_rollback_restart(trans, status) != 0)
+					? -1 : 0;
+			} else {
+				result = fbt_rollback_retaining(trans->fbt_transaction, status);
+			}
 			break;
 		case (COMMIT | RETAIN):
 			/* Issue #586: isc_commit_retaining retains all relation locks
@@ -1428,8 +1488,9 @@ static void _php_fbird_trans_end(INTERNAL_FUNCTION_PARAMETERS, int commit)
 	}
 
 	/* Clear handle for non-retained operations BEFORE checking result.
-	 * The fbt_* functions ALWAYS delete the wrapper (even on error),
-	 * so we must clear our pointer to avoid dangling references.
+	 * fbt_commit()/fbt_rollback() do NOT delete the wrapper (it is
+	 * explicitly freed below via fbt_free), so we clear our pointer and
+	 * free here to avoid dangling references.
 	 * Fixes: #9, #10 - SIGSEGV due to use-after-free of transaction wrapper
 	 */
 	if ((commit & RETAIN) == 0) {

firebird.c

@@ -1284,6 +1284,7 @@ PHP_FUNCTION(fbird_gen_id)
 
 	/* Prepare the query via OO API */
 	stmt = fbs_prepare(FBG(master_instance), attachment, transaction_ptr,
+		NULL,  /* #593 */
 		query, (unsigned)strlen(query), SQL_DIALECT_CURRENT, status);
 	if (!stmt) {
 		_php_fbird_error(status);
@@ -1383,6 +1384,7 @@ PHP_FUNCTION(fbird_last_insert_id)
 	}
 
 	void *stmt = fbs_prepare(FBG(master_instance), attachment, transaction_ptr,
+		NULL,  /* #593 */
 		query, (unsigned)strlen(query), SQL_DIALECT_CURRENT, status);
 	if (!stmt) {
 		_php_fbird_error(status);