fix(#595,#596,#599): connect sync param, asan harness scope, batch registry sweep - #600
Merged
Conversation
…v, batch registry sweep #595: declare the sync (lock timeout) parameter that zpp has always accepted ('|ssssllsll' = 9 slots) on fbird_connect/fbird_pconnect - arginfo+stubs documented only 8, so debug builds fatalled on deep parses and an 8th positional arg silently landed in sync instead of flags. UPGRADING.md documents the positional-call migration. tests/fbird_connect_force_new.phpt now passes under php83-asan. #596: fbird_drop_db_003 BORKed under asan because LSAN's exit check fatally conflicts with run-tests --set-timeout ptrace on leak-bearing paths. Scoped detect_leaks=0 via --ENV-- (UAF detection stays armed). #599: extend the #594 transaction registry to fbird_batch resources - same raw backref, dereferenced by fbird_batch_execute(). fbird_batch gains a struct tag and moves out of the FB40 guard (POD decl only); enroll at create, unregister at both free exits, detach in the same sweep pass. batch_head zero-init added at every creation site alongside query_head (emalloc'd structs - the missing init caused a 24-failure regression caught by the full suite before push). Functional verification in php83-dev (FB5 client); note: no local asan+IBatch environment exists (php83-asan links FB3 client, CI sanitizers are build-only per RTLD_DEEPBIND) so the UAF path itself is covered by pattern-equivalence to the verified #594 query sweep. New test: tests/issue599_batch_survives_trans_death.phpt
This was referenced Aug 24, 2026
Closed
satwareAG-ironMike
added a commit
that referenced
this pull request
Aug 24, 2026
- fbird_transaction.c: the #599 batch-walk order bug (batch_head cleared before the walk reads it - #600 init-regex collateral) is documented with a jane: comment and DEFERRED to #603: activating the walk crashes fbird_batch_multitype_001 at process exit (simple case passes; the complex lifecycle needs its own root-cause). Walk left in the known-good dead state. - firebird_utils.cpp + fb_connection.hpp: unified #591/#593 banner - stale per-thread description removed; thread-model claim corrected to 'safe because PHP never manipulates one connection's statements from multiple threads concurrently' (was: ZTS-safe-by-construction, which overstated) - UPGRADING.md: lockstep-upgrade note for the firebird.so/pdo_fbird.so ABI boundary (old pdo .so + new firebird .so = PDO::prepare SIGSEGV, #602 class) Full suite php83-dev: 312/0.
satwareAG-ironMike
added a commit
that referenced
this pull request
Aug 24, 2026
…per-connection sweep registry (#601) * fix(#589,#597,#593): rollback_ret locks, drop_db tx orphan, ZTS-safe sweep registry #589: isc_rollback_retaining retains attachment-level relation locks exactly like isc_commit_retaining (#586) - probe: after a retaining rollback of DDL, a fresh SERIALIZABLE NOWAIT attachment blocked over metadata. Zero-open-cursor rollback_ret now hard-rolls-back + restarts with the stored TPB (_php_fbird_trans_rollback_restart), releasing the locks; cursor-bearing transactions keep true retaining semantics. #597: fbird_drop_db() frees live transaction handles BEFORE fbc_drop_database() while the attachment is still valid (rollbackNoThrow+delete via fbt_free) instead of orphaning the fb::Transaction wrappers afterwards (LSAN 40B each). issue591/issue582 run with full LSAN again (scoped detect_leaks=0 removed); only fbird_drop_db_003 keeps it for an unrelated residual init-path leak. #593: statement sweep registry moved from thread_local onto fb::Connection (stmt_head_) - whoever closes the connection sweeps its wrappers regardless of preparing thread (ZTS-safe by construction). fbs_prepare gains a connection_ptr parameter (service-API callers pass NULL - synchronous lifetimes). Service wrappers remain unregistered no-ops by design. New test: tests/issue589_rollback_ret_locks.phpt * fix(pdo): sync fbs_prepare signature into pdo_fbird header + enroll statements PR #601 session-4 changed fbs_prepare's signature (connection_ptr for the #593 per-connection sweep registry) but pdo_fbird carries its OWN copies of firebird_utils.h/php_fbird_includes.h. PDO compiled against the stale 8-param declaration while the main ext exported 9 params -> garbage args (sql=0x1f) -> SIGSEGV in pdo_fbird_handle_preparer on every pdo test, across all CI matrix cells. Fix: sync the header and pass H->fbc_conn at all three call sites - which also enrolls PDO statements in the #593 sweep registry (bonus: PDO prepared statements now get connection-death invalidation too). Found via full-CI-matrix failure; reproduced locally on php83-fb3-dev; FB3 full suite 358/0 after fix. * chore(test): --CLEAN-- section for #589 test (quality gate) * fix(review): PR #601 review findings + #602 header-parity gate Review findings applied: - UPGRADING #589: BLOB-handle invalidation warning (same hazard as #586; rollback_ret blob-loops must keep a cursor open) - fbird_inspection.c x3: comment rationale corrected - NULL is safe because the statement is freed synchronously in-function, NOT because it is a 'service attachment' (these use the user's attachment; the wrong rationale invited unsafe copies of the pattern) - fbird_transaction.c: stale comment claiming fbt_* 'ALWAYS delete the wrapper' contradicted by implementations and load-bearing-adjacent for rollback_restart's failure path #602 acceptance criteria (review finding 3 - landed here per reviewer): - scripts/check-header-parity.sh: fails when firebird_utils.h (ABI- load-bearing prototypes) or php_fbird_includes.h (struct layouts) drift between main ext and pdo_fbird copies - pdo_fbird/php_fbird_includes.h reconciled to a byte-identical copy (was generations stale; safe because pdo touches only opaque handles - verified by FB3 full suite 358/0 after sync) - code-quality.yml runs the gate alongside check-stubs-sync Ponytail -30 twin-merge skipped deliberately: repo convention is documented per-issue twins (commit_restart/rollback_restart). * fix(review): PR #601 round-2 findings - fbird_transaction.c: the #599 batch-walk order bug (batch_head cleared before the walk reads it - #600 init-regex collateral) is documented with a jane: comment and DEFERRED to #603: activating the walk crashes fbird_batch_multitype_001 at process exit (simple case passes; the complex lifecycle needs its own root-cause). Walk left in the known-good dead state. - firebird_utils.cpp + fb_connection.hpp: unified #591/#593 banner - stale per-thread description removed; thread-model claim corrected to 'safe because PHP never manipulates one connection's statements from multiple threads concurrently' (was: ZTS-safe-by-construction, which overstated) - UPGRADING.md: lockstep-upgrade note for the firebird.so/pdo_fbird.so ABI boundary (old pdo .so + new firebird .so = PDO::prepare SIGSEGV, #602 class) Full suite php83-dev: 312/0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Session-3 queue batch:
Verification
Known limitation
No local ASAN+IBatch environment exists (asan image links FB3 client; CI sanitizers are build-only per RTLD_DEEPBIND) - #599 UAF-path coverage relies on pattern equivalence with the verified #594 query sweep.