feat(mysqlnd): send COM_RESET_CONNECTION in restart_psession by ericnorris · Pull Request #21857 · php/php-src

ericnorris · 2026-04-23T19:37:37Z

Closes #20225. I'm not sure if adding this to PDO is a step too far; we could limit this to just mysqlnd if necessary.

Disclosure: I've used Claude to write the commit (the commit message is mostly mine), but take full responsibility for the changes and have reviewed it to the best of my ability.

Before this commit, persistent MySQL connection reuse relied on COM_CHANGE_USER to reset server-side session state in mysqli. PDO MySQL did not reset server-side state at all, it only pinged to check liveness, leaving user variables, temporary tables, and transaction state from previous requests intact.

After this commit, both mysqli and PDO MySQL send COM_RESET_CONNECTION when reusing a persistent connection, which resets all server-side session state without re-authentication.

We've implemented this by adding a reset_connection command to mysqlnd's command layer. It's similar to the existing ping command, since there's no payload. We now call it from restart_psession, which we've also changed to return enum_func_status so callers can handle failures.

In mysqli, we've replaced the COM_CHANGE_USER call in the persistent reuse path with a call to restart_psession.

In PDO MySQL, we've replaced the mysql_ping in check_liveness with a call to restart_psession, which both verifies the connection is alive and resets session state in a single round trip. We've also removed a call to restart_psession on unconnected handles during initialization.

Before this commit, persistent MySQL connection reuse relied on COM_CHANGE_USER to reset server-side session state in mysqli. PDO MySQL did not reset server-side state at all; it only pinged to check liveness, leaving user variables, temporary tables, and transaction state from previous requests intact. After this commit, both mysqli and PDO MySQL send COM_RESET_CONNECTION when reusing a persistent connection, which resets all server-side session state without re-authentication. We've implemented this by adding a reset_connection command to mysqlnd's command layer. It's similar to the existing ping command, since there's no payload. We now call it from restart_psession, which we've also changed to return enum_func_status so callers can handle failures. In mysqli, we've replaced the COM_CHANGE_USER call in the persistent reuse path with a call to restart_psession. In PDO MySQL, we've replaced the mysql_ping in check_liveness with a call to restart_psession, which both verifies the connection is alive and resets session state in a single round trip. We've also removed a call to restart_psession on unconnected handles during initialization.

The initial approach reset PDO MySQL persistent connections by sending COM_RESET_CONNECTION from check_liveness. This should have been safe because check_liveness is only ever used by the persistent connection code, but it is somewhat confusing from a behavior perspective - why should a liveness check actually mutate the connection? Instead, I've added a dedicated reset_connection hook to the driver method table (pdo_dbh_methods). On persistent reuse, PDO core calls reset_connection when the driver provides it, and falls back to check_liveness (the existing behavior) otherwise. A successful reset also confirms liveness, so the two are never both invoked. PDO MySQL implements the hook (mysqlnd builds only) via restart_psession, and check_liveness is restored to its plain ping. The other bundled PDO drivers leave the hook NULL and keep their existing behavior.

…reset-connection

NattyNarwhal

Some things I noticed from skimming...

NattyNarwhal · 2026-07-02T17:16:25Z

 	pdo_pgsql_fetch_error_func,
 	pdo_pgsql_get_attribute,
 	pdo_pgsql_check_liveness,	/* check_liveness */
+	NULL,	/* reset_connection */


The current pdo_pgsql_check_liveness actually calls PQreset if it does get a bad connection, which is more semantically like reset_connection.

Interesting! I don't think that changes anything here though; it doesn't call PQreset for a good connection and so it wouldn't necessarily reset.

A persistent PDO connection can be shared by several open handles in the same request: two `new PDO(sameDSN, [PERSISTENT])` resolve to one underlying connection. Resetting it every time a handle is constructed would wipe session state — open transactions, temporary tables, user variables — out from under a handle that is still using it. This commit changes the persistent connection code to gate the reset on the connection being idle: when its refcount is 1 we hold the only reference, so it is safe to reset (which also confirms liveness) and, on failure, discard it. Note that previously the code only discarded the connection by marking the resource closed and decrementing the refcount — nothing ever ran the closer — and thus the PDO handle, and potentially the connection, leaked. Now, we actually remove the connection from the persistent list, so we both close the connection (if necessary) and ensure that the handle is cleaned up. If the refcount is greater than 1, we follow the existing behavior of only running a liveness check.

Adding reset_connection after check_liveness shifts every member after it. An out-of-tree driver that recompiles against the new struct without updating its positional initializer would then map each value onto the wrong member, so PDO would call the wrong function pointers. Appending reset_connection at the end keeps those values aligned, leaving only the new, unset member NULL, which falls back to check_liveness. An un-updated initializer still warns under -Wmissing-field-initializers regardless of placement; that is why the in-tree drivers get an explicit trailing NULL. Placement only affects whether the remaining values stay correctly aligned.

kamazee · 2026-07-28T19:45:01Z

I'm not sure if adding this to PDO is a step too far; we could limit this to just mysqlnd if necessary.

I would argue that adding this to PDO is a reasonable thing (it makes resetting connection dead simple) but doing it unconditionally actually is a step too far.

Initially I thought that breaking persistent connections with an older versions of databases is not really reasonable in this case but then few other things popped up.

For about 2 decades (at least since 2003) PHP advised to do the cleanup in a shutdown function. I guess there is code out in the wild that does exactly that; a colleague of mine once advocated for resetting the state in the beginning of processing a request when using a persistent connection rather than in the end to be sure that it happens. We don't know what exactly those functions do, and if COM_RESET_CONNECTION is issued unconditionally, such code will appear subtly broken -- in a way that is harder to detect or predict that when there are some syntactic changes that often can be fixed automatically.
When looking at how it's done elsewhere, I noticed this. It turns out, the behavior varies between MariaDB versions and between MariaDB and MySQL, and with that making it unconditional hurts even more.

So, if you're up to revisiting it, I'd be glad to help if I can. Well, the RFC is passing pretty much unanimously (but I'm not sure the voters fully realize the impact because it's much more than minimal supported version), I'll try to squeeze another RFC in but the time is ticking out, so not sure if it's feasible and at the same time feel that we should do better for those who use persistent connections.

ericnorris · 2026-07-28T20:31:58Z

For about 2 decades (at least since 2003) PHP advised to do the cleanup in a shutdown function. I guess there is code out in the wild that does exactly that; a colleague of mine once advocated for resetting the state in the beginning of processing a request when using a persistent connection rather than in the end to be sure that it happens. We don't know what exactly those functions do, and if COM_RESET_CONNECTION is issued unconditionally, such code will appear subtly broken -- in a way that is harder to detect or predict that when there are some syntactic changes that often can be fixed automatically.

A shutdown function is not guaranteed to run, so I don't think it's an acceptable equivalent.

Additionally, it is not clear to me how sending COM_RESET_CONNECTION unconditionally would break code that resets the state at the beginning of a request. If you're resetting state in userland - although it's also unclear to me how you would even do that without the low-level command, I'll assume it's possible - after we sent COM_RESET_CONNECTION, you'd just reset it twice. Could you perhaps detail a concrete example where doubly resetting would break something?

When looking at how it's done elsewhere, I noticed this. It turns out, the behavior varies between MariaDB versions and between MariaDB and MySQL, and with that making it unconditional hurts even more.

The linked issue in MySQL claims it is not a bug. Additionally, the linked issue also notes that there is a very easy workaround, which I agree with. It makes sense to me that resetting the connection would reset the charset, which you would then be expected to set.

I don't think you've explicitly said this, but a coworker suggested that I should point this out: if there is a demonstrable difference between a fresh connection and a persistent connection, I would consider that a bug. If we end up needing to do some sort of charset re-initialization to maintain the charset from the DSN, I would consider that acceptable.

So, if you're up to revisiting it, I'd be glad to help if I can. Well, the RFC is passing pretty much unanimously (but I'm not sure the voters fully realize the impact because it's much more than minimal supported version), I'll try to squeeze another RFC in but the time is ticking out, so not sure if it's feasible...

I am sorry, but I am not open to revisiting it. I appreciate that you are passionate about this topic, but I simply disagree that making this optional is the "correct" choice. I also believe that voters have been properly informed by the same discussion that we had on the internals mailing list.

and at the same time feel that we should do better for those who use persistent connections.

It may be helpful to explain a bit about why I'm interested in this patch in the first place - at Etsy, we have avoided using persistent connections specifically because of the current unsafe behavior. From my point of view, we're actually doing the best by our users if we maintain PHP's shared-nothing request model. I have yet to see evidence that this would cause wide spread breakages or difficulties to convince me otherwise.

ericnorris · 2026-07-30T16:12:15Z

The vote to include this in PHP 8.6 has passed: https://wiki.php.net/rfc/min_supported_versions_php_8_6.

kamazee · 2026-07-30T17:50:41Z

@ericnorris congrats! I really appreciate you pushing it forward. Up until my first reply on the subject into the internals list, I didn't even know this existed, and for me, too, at my day job this is going to be the enabler for using persistent connections. Thanks!

I still disagree with the way this new convenience is introduced, though, so I'll start another RFC to make it optional for the reasons stated above plus one I dug up yesterday when working on a demo setup: there are databases that use mysql-ish wire protocol, and they might not support COM_RESET_CONNECTION. I tested ClickHouse and SingleStore; while the latter seems to implement it, the former doesn't, so it also just breaks persistent connections for ClickHouse over MySQL protocol.

I do believe that us, who want to use it, should be able to -- by opting in; those who don't (for whatever reason), should work exactly in the way they have and there's no good reason to break their apps.

NattyNarwhal · 2026-07-30T18:13:26Z

I think we want to avoid adding more knobs when possible.

Does Clickhouse return an error code for commands it doesn't recognize? It may be possible to send COM_RESET_CONNECTION when connecting for the first time, persist if it worked (assuming it returns an error), then use that to determine if it should use it when fetching the connection later. That would also cover legacy MySQL (though I don't think that matters anymore), but it does complicate things a bit (especially on the PDO side, where it's likely another method for like can_reset_connection or adding a different return code to fall back to check_liveness); the end user wouldn't have to worry about it though.

kamazee · 2026-07-30T19:33:22Z

I think we want to avoid adding more knobs when possible.

Generally, I agree: if it was something new rather than a substantial change, it would make sense to default to something and then see if there's a demand for something else rather than add more code paths. The suggestion is also good: it indeed should cover issues with ClickHouse and older databases.

On the other hand, complicating this logic is barely better then letting a developer decide what they want for their application.
As a person who's been behind upgrading massive PHP applications many times over last 2 decades, I would prefer to first upgrade PHP, deploy it to production, make sure I don't need to roll back, get rid of the compatibility layers, and then add the option, taking one step at a time (interacting with databases is a big deal itself). Please let developers decide!
This can be made on by default later on (in PHP 9 or something) but even then, there must be a way to turn it off.

Lack of another knob breaks the behavior that has been there for over 2 decades: official documentation has recommended implementing a custom logic for resetting connection's state for literally more than 20 years, we don't know how many people did it, we don't know what exactly they did, and the new way that is unconditionally on can break their application is a subtle way that is hard to predict unless they are very well tested (which is not always the case) -- I'm preparing a demo, will share in the internals list no later than in few days; this is a main reason why I'm sure this is not the case to save on a knob and will try to squeeze in another RFC before we're past beta stage.

Does Clickhouse return an error code for commands it doesn't recognize?

It does as far as I can tell; can't give more details at the moment (can get back to it later if needed)

ericnorris · 2026-07-30T22:00:04Z

there are databases that use mysql-ish wire protocol, and they might not support COM_RESET_CONNECTION. I tested ClickHouse and SingleStore; while the latter seems to implement it, the former doesn't, so it also just breaks persistent connections for ClickHouse over MySQL protocol.

I could argue that persistent connections to ClickHouse over MySQL are already "broken" today, since they are not properly reset. The difference you and I have is in what we believe is "correct", and I have been unable to convince you otherwise.

I would like to note that, for persistent connections created via mysqlnd, we have already been issuing COM_CHANGE_USER when reusing a connection. Thus, resetting the connection in some form has a precedent for MySQL persistent connections, just not via PDO. If a user was using mysqlnd persistent connections to ClickHouse, they could already have this problem - does ClickHouse support COM_CHANGE_USER? If it doesn't, are we saying that PDO MySQL needs to be more compatible and lenient than myslqnd MySQL, even though that's what PDO uses under the hood? That feels like a strange argument to me, and one I would vote against.

Lack of another knob breaks the behavior that has been there for over 2 decades: official documentation has recommended implementing a custom logic for resetting connection's state for literally more than 20 years, we don't know how many people did it, we don't know what exactly they did, and the new way that is unconditionally on can break their application is a subtle way that is hard to predict unless they are very well tested (which is not always the case)

I have yet to see a concrete example of how this would break; I asked you for one in my previous response and yet you are repeating this statement without evidence. Even if you were to demonstrate a breakage, users are not forced to upgrade to the latest PHP, nor are they forced to use persistent connections. I would personally need to be convinced that this was so problematic as to outweigh the common good of this working correctly by default for everyone.

@kamazee, respectfully, this argument is starting to feel circular. Can we keep this pull request focused on making the best possible implementation of the RFC that the mailing list approved? It sounds like you would like to open a new RFC, and I am happy to debate the merits of your points there, but I would like to move forward with merging this.

TimWolla

Not an expert, but the changes to the C code look good to me. Did not look into the tests. Consider this a soft-approval.

onyn · 2026-07-31T09:25:19Z

there are databases that use mysql-ish wire protocol, and they might not support COM_RESET_CONNECTION. I tested ClickHouse and SingleStore

Why should we bother about other databases? I suppose this is bug in ClickHouse/SingleStore and bug report should be filed there. Not here.

I still disagree with the way this new convenience is introduced...

I also want to see smoother transition to COM_RESET_CONNECTION. Something like proposed here: https://externals.io/message/130704#130958 (fallback to COM_CHANGE_USER and E_WARNING when this fallback hit).

I agree that new options (ini or PDO) not needed here. It's easy to introduce new, but hard to get rid later.

...at Etsy, we have avoided using persistent connections specifically because of the current unsafe behavior.

I waited this for years. Finally pconnections become usable in PDO!

It makes sense to me that resetting the connection would reset the charset, which you would then be expected to set.

This is unexpected and serious problem. It can be easily avoided by explicitly issuing "SET NAMES" sql statement, but before I figure out that I should do this, some of my data will break.

ericnorris · 2026-07-31T15:28:33Z

there are databases that use mysql-ish wire protocol, and they might not support COM_RESET_CONNECTION. I tested ClickHouse and SingleStore

Why should we bother about other databases? I suppose this is bug in ClickHouse/SingleStore and bug report should be filed there. Not here.

Agreed.

I still disagree with the way this new convenience is introduced...

I also want to see smoother transition to COM_RESET_CONNECTION. Something like proposed here: https://externals.io/message/130704#130958 (fallback to COM_CHANGE_USER and E_WARNING when this fallback hit).

The RFC approved moving forward without a deprecation notice or warning, so I am not going to pursue that. Users that are unwilling or unable to use a recent version of MySQL (etc.) will simply always get a fresh connection. We should obviously document this as a part of our UPGRADING notes, of course.

It makes sense to me that resetting the connection would reset the charset, which you would then be expected to set.

This is unexpected and serious problem. It can be easily avoided by explicitly issuing "SET NAMES" sql statement, but before I figure out that I should do this, some of my data will break.

I am looking into this and any other re-initialization we should do now.

Before this commit, persistent connections had their character set and the state of any init commmands reset to the connection defaults upon reuse. Furthermore, since since mysqlnd tries to track the character set, mysqlnd would think it was using the desired character set when it in fact was not. After this commit, restart_psession now re-applies the setup a fresh connection establishes. It sets the character set when it differs from the one the server advertised in its greeting (conn->greet_charset) to handle when the user selected a non-default character set via the DSN (charset=), via mysqli_options(MYSQLI_SET_CHARSET_NAME), or via set_charset(). It also executes any configured init commands.

This commit reorganizes the persistent-reuse refcount check to make the `else` more clearly a block where `refcount > 1`, so the purpose of the next check (`if (pdbh->methods->check_liveness ...`) is also more clear.

ericnorris · 2026-08-03T15:15:21Z

This PR is ready for another review; I've addressed the PR comment about the else block and added code to re-initialize the character set and any init commands when reusing a connection.

The ci/circleci: arm failure seems unrelated, but I can't re-run the test.

TimWolla · 2026-08-03T19:13:44Z

+						if (pdbh->methods->check_liveness
+								&& FAILURE == (pdbh->methods->check_liveness)(pdbh)) {


Suggested change

if (pdbh->methods->check_liveness

&& FAILURE == (pdbh->methods->check_liveness)(pdbh)) {

if (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) {

This will keep git diff -w more obvious.

TimWolla · 2026-08-03T19:18:29Z

+if (!$link)
+    printf("[001] Cannot connect\n");
+
+mysqli_set_charset($link, 'latin1');


I'd argue that this charset should be reset, because to my understanding it is set after the connection is already established. This manual call thus would be present even for reused connections.

Technically, setting charset cannot be done during connection, so every change of charset happens after the connection has been established.

TimWolla · 2026-08-03T19:20:29Z

Perhaps extend this by opening a transaction and checking if the transaction still is active afterwards?

TimWolla · 2026-08-03T19:21:02Z

+<?php
+require_once __DIR__ . '/inc/mysql_pdo_test.inc';
+
+$dsn  = MySQLPDOTest::getDSN() . ';charset=latin1';


This one however is part of the DSN and thus should be restored automatically.

kamil-tekiela · 2026-08-03T19:52:42Z

+}
+mysqli_free_result($res);
+
+/* SHOW TABLES never lists temporary tables, so probe by selecting from it */


Copy-pasta? Where is the SHOW TABLES? This comment appears here 3 times

ericnorris requested review from SakiTakamachi, bukka and kamil-tekiela as code owners April 23, 2026 19:37

github-actions Bot added Extension: mysqli Extension: mysqlnd Extension: pdo_mysql ABI break labels Apr 23, 2026

ericnorris mentioned this pull request Apr 24, 2026

Send COM_RESET_CONNECTION in mysqlnd_conn_data::restart_psession #20225

Open

ericnorris added 2 commits June 26, 2026 16:26

Merge remote-tracking branch 'upstream/master' into feat-mysqlnd-com-…

d684213

…reset-connection

ericnorris requested review from NattyNarwhal and devnexen as code owners June 26, 2026 15:35

github-actions Bot added Extension: pdo (core) Extension: pdo_dblib Extension: pdo_firebird Extension: pdo_odbc Extension: pdo_pgsql Extension: pdo_sqlite labels Jun 26, 2026

NattyNarwhal reviewed Jul 2, 2026

View reviewed changes

ericnorris added 2 commits July 3, 2026 11:36

TimWolla reviewed Jul 31, 2026

View reviewed changes

Comment thread ext/pdo/pdo_dbh.c Outdated

ericnorris added 2 commits August 3, 2026 09:59

refactor(pdo): address @TimWolla's comment

c8f7395

This commit reorganizes the persistent-reuse refcount check to make the `else` more clearly a block where `refcount > 1`, so the purpose of the next check (`if (pdbh->methods->check_liveness ...`) is also more clear.

TimWolla reviewed Aug 3, 2026

View reviewed changes

kamil-tekiela reviewed Aug 3, 2026

View reviewed changes

		if (pdbh->methods->check_liveness
		&& FAILURE == (pdbh->methods->check_liveness)(pdbh)) {

	if (pdbh->methods->check_liveness
	&& FAILURE == (pdbh->methods->check_liveness)(pdbh)) {
	if (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) {

Uh oh!

Conversation

ericnorris commented Apr 23, 2026

Uh oh!

NattyNarwhal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kamazee commented Jul 28, 2026

Uh oh!

ericnorris commented Jul 28, 2026

Uh oh!

ericnorris commented Jul 30, 2026

Uh oh!

kamazee commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NattyNarwhal commented Jul 30, 2026

Uh oh!

kamazee commented Jul 30, 2026

Uh oh!

ericnorris commented Jul 30, 2026

Uh oh!

TimWolla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

onyn commented Jul 31, 2026

Uh oh!

ericnorris commented Jul 31, 2026

Uh oh!

ericnorris commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kamazee commented Jul 30, 2026 •

edited

Loading

ericnorris commented Aug 3, 2026 •

edited

Loading