Fix leak of driver_params argument of PDOStatement::bindParam() and bindColumn() - #23463
Open
iliaal wants to merge 1 commit into
Open
Fix leak of driver_params argument of PDOStatement::bindParam() and bindColumn()#23463iliaal wants to merge 1 commit into
iliaal wants to merge 1 commit into
Conversation
iliaal
force-pushed
the
fix/pdo-bindparam-driverparams-leak-84
branch
from
August 25, 2026 23:13
fffa83c to
8199d65
Compare
NickSdot
reviewed
Aug 26, 2026
| $obj = new C(); | ||
| try { | ||
| $stmt->bindParam(1, $obj, PDO::PARAM_STR, 0, $dp); | ||
| } catch (Error $e) { |
Contributor
There was a problem hiding this comment.
Sorry to use this to ask a question, but trying to understand the rational. Do you mind to make me understand why you sometimes decide not printing anything? Wouldn't the same argument as in #23041 (comment) apply here?
Contributor
Author
There was a problem hiding this comment.
The loop would print it 20k times. Pinned class and message once.
register_bound_param() took a reference on the driver_params argument but never released it: really_register_bound_param() ADDREFs another reference for the bound-params hash, yet its early failure paths after that point (rewrite_name_to_position(), PDO_PARAM_EVT_NORMALIZE hook) and every return path of register_bound_param() itself dropped only param.parameter, leaking one or two references per call. 200k failing bindParam() calls grow memory by ~128MB. The transient copy is now released on both failure and success, and really_register_bound_param() releases its hash-bound reference on early failures; the PDO_PARAM_EVT_ALLOC hook failure path already released it via the hash dtor. Sibling audit: bindValue() and execute()'s input_params loop leave driver_params undefined so the new releases are no-ops there. Closes phpGH-23463
iliaal
force-pushed
the
fix/pdo-bindparam-driverparams-leak-84
branch
from
August 26, 2026 12:27
8199d65 to
6b819bd
Compare
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
2 participants
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.
PDOStatement::bindParam() and bindColumn() leak references on the driver_params argument: register_bound_param() never releases its transient copy, and really_register_bound_param() drops its hash-bound reference on neither of its two early failure returns, so 200k failing bindParam() calls grow memory by about 128MB and even successful calls leak one reference each. Both paths now release what they take; the EVT_ALLOC hook failure needed no change because deleting the hash entry frees it through param_dtor().