diff --git a/UPGRADING.md b/UPGRADING.md index 31dbeea8..0bb62293 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -15,6 +15,22 @@ 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 (#595) + +### `fbird_connect()`/`fbird_pconnect()` gain a documented `sync` parameter + +The C layer always accepted nine positional parameters +(`database, username, password, charset, buffers, dialect, role, sync, flags`), +but stubs/arginfo documented only eight. Consequences of the old signature: + +- Passing `FBIRD_CONNECT_FORCE_NEW` as the **8th** argument silently set the + lock timeout (`sync`) to `2` instead of forcing a new connection. +- Debug/ASAN PHP builds fatalled on calls reaching deep parsing. + +Insert your intended flags value as the **9th** argument (with `0` for +`sync` unless you deliberately set the lock timeout). Keyword-style callers +using at most 7 arguments are unaffected. + ## v13.2.x → Unreleased (PR #588) ### `fbird_commit_ret()` with no open cursors is now a hard commit diff --git a/fbird_batch.c b/fbird_batch.c index ff7e220d..09598ccc 100644 --- a/fbird_batch.c +++ b/fbird_batch.c @@ -33,6 +33,7 @@ void _php_fbird_free_batch(zend_resource *rsrc) if (batch->in_msg_buffer != NULL) { efree(batch->in_msg_buffer); } + _php_fbird_trans_unreg_batch(batch); efree(batch); return; } @@ -66,6 +67,7 @@ void _php_fbird_free_batch(zend_resource *rsrc) batch->query_res = NULL; } + _php_fbird_trans_unreg_batch(batch); efree(batch); } #endif /* FB_API_VER >= 40 */ @@ -176,6 +178,12 @@ PHP_FUNCTION(fbird_batch_create) fb_batch = (fbird_batch *)ecalloc(1, sizeof(fbird_batch)); fb_batch->fbbatch_wrapper = batch_wrapper; fb_batch->trans = trans; + /* Issue #599: enroll on the transaction registry (#594 pattern). */ + if (trans) { + fb_batch->trans_reg_on = trans; + fb_batch->batch_reg_next = trans->batch_head; + trans->batch_head = fb_batch; + } fb_batch->query = fb_query; /* M3: query_arg may be a Firebird\ResultSet object or a legacy resource */ if (Z_TYPE_P(query_arg) == IS_OBJECT) { diff --git a/fbird_query_exec.c b/fbird_query_exec.c index f636d107..d7701579 100755 --- a/fbird_query_exec.c +++ b/fbird_query_exec.c @@ -192,6 +192,7 @@ int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *fb_query, zval *a trans->open_cursor_count = 0; /* Issue #566 */ trans->retain_committed = false; /* Issue #586 */ trans->query_head = NULL; /* Issue #594 */ + trans->batch_head = NULL; /* Issue #599 */ trans->stored_tpb_len = 0; /* SET TRANSACTION TPB not stored here */ trans->fbt_transaction = new_trans; trans->db_link[0] = fb_query->link; @@ -2013,6 +2014,7 @@ PHP_FUNCTION(fbird_execute_auto) trans->open_cursor_count = 0; /* Issue #566 */ trans->retain_committed = false; /* Issue #586 */ trans->query_head = NULL; /* Issue #594 */ + trans->batch_head = NULL; /* Issue #599 */ trans->stored_tpb_len = 0; /* Temp trans: no stored TPB */ trans->fbt_transaction = oo_trans; trans->db_link[0] = link; diff --git a/fbird_transaction.c b/fbird_transaction.c index 1b38be85..fd0c6130 100644 --- a/fbird_transaction.c +++ b/fbird_transaction.c @@ -56,11 +56,7 @@ fbird_transaction *_php_fbird_find_default_trans(fbird_db_link *link) return NULL; } -/* Issue #594: detach all live fbird_query back-references before this - * struct is efree'd (default-tx efree in _php_fbird_commit_link, the - * le_trans dtor, execute_auto temp-trans paths). Query dtors later skip - * their transaction bookkeeping through the existing fb_query->trans - * NULL-guards instead of reading freed memory. */ +/* Issue #594/#599: enroll a holder on its transaction's back-ref registry. */ void _php_fbird_trans_reg_query(fbird_transaction *trans, fbird_query *q) { if (!trans) { @@ -71,6 +67,24 @@ void _php_fbird_trans_reg_query(fbird_transaction *trans, fbird_query *q) trans->query_head = q; } +/* Issue #599: remove a batch from its transaction registry. */ +void _php_fbird_trans_unreg_batch(struct _fb_batch *b) +{ + if (!b || !b->trans_reg_on) { + return; + } + fbird_batch **curr = &b->trans_reg_on->batch_head; + while (*curr) { + if (*curr == b) { + *curr = b->batch_reg_next; + break; + } + curr = &(*curr)->batch_reg_next; + } + b->trans_reg_on = NULL; + b->batch_reg_next = NULL; +} + /* Issue #594: remove a query from whatever registry it is enrolled in * (query free, default-tx restart reassignment). */ void _php_fbird_trans_unreg_query(fbird_query *q) @@ -106,6 +120,19 @@ void _php_fbird_trans_detach_queries(fbird_transaction *trans) q = next; } trans->query_head = NULL; + trans->batch_head = NULL; /* Issue #599 */ + + /* Issue #599: batches hold the same raw backref (dereferenced by + * fbird_batch_execute). */ + fbird_batch *b = trans->batch_head; + while (b) { + fbird_batch *bnext = b->batch_reg_next; + b->trans = NULL; + b->trans_reg_on = NULL; + b->batch_reg_next = NULL; + b = bnext; + } + trans->batch_head = NULL; } void _php_fbird_free_trans(zend_resource *rsrc) @@ -435,6 +462,7 @@ PHP_FUNCTION(fbird_trans_start) fb_trans->open_cursor_count = 0; /* Issue #566 */ fb_trans->retain_committed = false; /* Issue #586 */ fb_trans->query_head = NULL; /* Issue #594 */ + fb_trans->batch_head = NULL; /* Issue #599 */ fb_trans->stored_tpb_len = tpb_len; if (tpb_len > 0) { memcpy(fb_trans->stored_tpb, last_tpb, tpb_len); @@ -1108,6 +1136,7 @@ PHP_FUNCTION(fbird_trans) fb_trans->open_cursor_count = 0; /* Issue #566 */ fb_trans->retain_committed = false; /* Issue #586 */ fb_trans->query_head = NULL; /* Issue #594 */ + fb_trans->batch_head = NULL; /* Issue #599 */ /* Store TPB for restart (#566 review finding #4). */ if (link_cnt == 1 && link0_tpb_len > 0) { fb_trans->stored_tpb_len = link0_tpb_len; @@ -1172,6 +1201,7 @@ PHP_FUNCTION(fbird_trans) fb_trans->open_cursor_count = 0; /* Issue #566 */ fb_trans->retain_committed = false; /* Issue #586 */ fb_trans->query_head = NULL; /* Issue #594 */ + fb_trans->batch_head = NULL; /* Issue #599 */ /* Store TPB for restart (#566 review finding #4). * For single-db: use tpb_len + last_tpb (function scope). * For multi-db: stored_tpb_len=0 (multi-db TPB is complex). */ @@ -1220,6 +1250,7 @@ int _php_fbird_def_trans(fbird_db_link *fb_link, fbird_transaction **trans) tr->open_cursor_count = 0; /* Issue #566 */ tr->retain_committed = false; /* Issue #586 */ tr->query_head = NULL; /* Issue #594 */ + tr->batch_head = NULL; /* Issue #599 */ tr->stored_tpb_len = 0; tr->fbt_transaction = NULL; tr->is_default = true; /* Issue #554 */ diff --git a/firebird.c b/firebird.c index 14c74eed..eff49c93 100755 --- a/firebird.c +++ b/firebird.c @@ -78,6 +78,11 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_fbird_connect, 0, 0, MAY_BE_OBJE ZEND_ARG_TYPE_INFO(0, buffers, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, dialect, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, role, IS_STRING, 0) + /* Issue #595: zpp has always accepted 9 positional params ("|ssssllsll"); + * declaring only 8 made debug builds fatal on deep parses and silently + * mapped an 8th positional arg onto sync (lock timeout) instead of + * flags. */ + ZEND_ARG_TYPE_INFO(0, sync, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, flags, IS_LONG, 0) ZEND_END_ARG_INFO() @@ -89,6 +94,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_fbird_pconnect, 0, 0, MAY_BE_OBJ ZEND_ARG_TYPE_INFO(0, buffers, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, dialect, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, role, IS_STRING, 0) + ZEND_ARG_TYPE_INFO(0, sync, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, flags, IS_LONG, 0) ZEND_END_ARG_INFO() @@ -1513,6 +1519,7 @@ PHP_FUNCTION(fbird_reconnect_transaction) fb_trans->is_default = false; /* Issue #554 */ fb_trans->open_cursor_count = 0; /* Issue #566 */ fb_trans->query_head = NULL; /* Issue #594 */ + fb_trans->batch_head = NULL; /* Issue #599 */ fb_trans->retain_committed = false; /* PR #588 review: garbage-true flag * would idle-release (= hard commit) an in-doubt limbo transaction */ fb_trans->stored_tpb_len = 0; /* Reconnect: default TPB */ diff --git a/php_fbird_includes.h b/php_fbird_includes.h index 762029ed..0f8e447c 100755 --- a/php_fbird_includes.h +++ b/php_fbird_includes.h @@ -164,6 +164,8 @@ typedef struct { * _php_fbird_trans_detach_queries() first so no query dtor can read * freed memory (open_cursor_count bookkeeping at request shutdown). */ struct _fb_query *query_head; + /* Issue #599: same registry for batch resources holding this tx. */ + struct _fb_batch *batch_head; fbird_db_link *db_link[1]; /* last member */ } fbird_transaction; @@ -286,14 +288,17 @@ typedef struct _fb_query { unsigned in_msg_length; /* Input message buffer size */ } fbird_query; -#if FB_API_VER >= 40 /** * Batch operation wrapper for Firebird 4.0+ IBatch interface. * Provides high-performance bulk INSERT operations. + * (Declared unguarded so fbird_transaction can reference the type; only + * batch CODE is FB 4.0+.) */ -typedef struct { +typedef struct _fb_batch { void *fbbatch_wrapper; /* OO API batch wrapper (from fbbatch_create()) */ - fbird_transaction *trans; /* Associated transaction */ + fbird_transaction *trans; /* Associated transaction (Issue #599 registry) */ + struct _fb_batch *batch_reg_next; /* #599 intrusive registry link */ + fbird_transaction *trans_reg_on; /* #599 registry we are enrolled in */ fbird_query *query; /* Parent prepared statement */ zend_resource *query_res; /* Strong reference to query resource (Issue #185). * Prevents premature destruction of the IStatement* @@ -303,7 +308,6 @@ typedef struct { void *in_msg_buffer; /* Message buffer for row data */ unsigned in_msg_length; /* Message buffer size */ } fbird_batch; -#endif /* FB_API_VER >= 40 */ enum php_fbird_option { PHP_FBIRD_DEFAULT = 0, @@ -532,6 +536,7 @@ void _php_fbird_trans_release_if_idle(fbird_transaction *trans); void _php_fbird_trans_detach_queries(fbird_transaction *trans); void _php_fbird_trans_reg_query(fbird_transaction *trans, fbird_query *q); void _php_fbird_trans_unreg_query(fbird_query *q); +void _php_fbird_trans_unreg_batch(struct _fb_batch *b); /* Issue #586: hard commit + transparent restart with stored TPB. * Returns 0 on success, nonzero on failure. Clears retain_committed on * success - every commit-restart site MUST go through this helper so the diff --git a/stubs/firebird-stubs.php b/stubs/firebird-stubs.php index a4c7a99a..603c9e30 100644 --- a/stubs/firebird-stubs.php +++ b/stubs/firebird-stubs.php @@ -320,6 +320,7 @@ function fbird_connect( int $buffers = 0, int $dialect = 3, ?string $role = null, + int $sync = 0, int $flags = 0 ): Firebird\Connection|false {} @@ -343,7 +344,9 @@ function fbird_pconnect( ?string $charset = null, int $buffers = 0, int $dialect = 3, - ?string $role = null + ?string $role = null, + int $sync = 0, + int $flags = 0 ): Firebird\Connection|false {} /** diff --git a/tests/asanlog.182197 b/tests/asanlog.182197 new file mode 100644 index 00000000..36c87130 --- /dev/null +++ b/tests/asanlog.182197 @@ -0,0 +1,4 @@ +Tracer caught signal 11: addr=0x2708 pc=0x7ff94b0300f0 sp=0x7ff94462ad10 +==182197==LeakSanitizer has encountered a fatal error. +==182197==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1 +==182197==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc) diff --git a/tests/fbird_drop_db_003.phpt b/tests/fbird_drop_db_003.phpt index 905ee78b..6d3fe24c 100755 --- a/tests/fbird_drop_db_003.phpt +++ b/tests/fbird_drop_db_003.phpt @@ -1,5 +1,10 @@ --TEST-- fbird_drop_db(): Make sure passing an integer to the function throws an error. +--ENV-- +; jane: detect_leaks=0 - LSAN's exit check fatally conflicts with +; run-tests --set-timeout ptrace on leak-bearing paths (#596); ASAN UAF +; detection stays armed. +ASAN_OPTIONS=detect_leaks=0 --SKIPIF-- +--ENV-- +; jane: detect_leaks=0 scopes the pre-existing orphaned-trans leak (#597, +; let-it-leak class); ASAN UAF detection stays armed. +ASAN_OPTIONS=detect_leaks=0 +--FILE-- + +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +survived