feat(mysqlnd): send COM_RESET_CONNECTION in restart_psession - #21857
feat(mysqlnd): send COM_RESET_CONNECTION in restart_psession#21857ericnorris wants to merge 11 commits into
Conversation
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.
NattyNarwhal
left a comment
NattyNarwhal
left a comment
There was a problem hiding this comment.
Some things I noticed from skimming...
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.
I would argue that adding this to 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.
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. |
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
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.
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.
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. |
|
The vote to include this in PHP 8.6 has passed: https://wiki.php.net/rfc/min_supported_versions_php_8_6. |
|
@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 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. |
|
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 |
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. 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.
It does as far as I can tell; can't give more details at the moment (can get back to it later if needed) |
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
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
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
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.
Try rebasing your branch onto the latest master (that should be done anyway to make sure it's actually testing against the current version). |
Good point! I've merged instead of rebasing; I hope that's okay. I imagine we'll squash or rebase this one final time at the end. |
TimWolla
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
Tests and code LGTM, but this is not my area of expertise.
| @@ -0,0 +1,93 @@ | |||
| --TEST-- | |||
| mysqli: reusing a persistent connection re-applies its connection config after reset | |||
There was a problem hiding this comment.
I haven't been immersed in this PR, but my first question is this: why do we need new mysqli tests? Doesn't this functionality already exist in mysqli?
There was a problem hiding this comment.
We need new mysqli tests because the previous functionality did not cause the character set to be reset.
|
|
||
| $host = 'p:' . $host; | ||
|
|
||
| // Open a persistent connection carrying both a charset and an init command. |
There was a problem hiding this comment.
Please collapse the code comment into the function name
| // COM_RESET_CONNECTION reverts the session to server defaults; mysqlnd should | ||
| // then re-apply the connection config, so a reused connection stays configured. |
There was a problem hiding this comment.
What is the purpose of this comment?
There was a problem hiding this comment.
This comment - which I wrote - is meant to explain the goal of this test in a little more detail. As noted, COM_RESET_CONNECTION will cause the connection to revert to server defaults (like the character set, or wiping away the results of any MYSQLI_INIT_COMMAND commands), and we want to ensure they are correctly restored when reusing the connection.
There was a problem hiding this comment.
Then it belongs in the test header, not here. But you used the word "connection config" and "configured" which isn't well-defined anywhere nor is generally used in this context. If you want to keep it, add it in the test header section and make it more explicit what it tests for.
There was a problem hiding this comment.
Understood. Do you have any better suggestion for the terminology to use around describing "the set of options specified at the time of connection" and "state that the user may have introduced post-connection". The former should always be preserved, the latter should always be reset.
There was a problem hiding this comment.
You can just list them out. The documentation does the same https://www.php.net/manual/en/mysqli.persistconns.php
"the set of options specified at the time of connection" -> It's good to list which options get restored. Be explicit.
There was a problem hiding this comment.
Doesn't that run the risk of "lying"? I thought it would be more descriptive to give a name to the set, and thus the set tested by the code can grow and shrink without needing to update the comment.
| return $link; | ||
| } | ||
|
|
||
| // Establish a persistent handle with the above configuration. |
There was a problem hiding this comment.
Please remove all unneeded comments. Clause loves to add comments.
There was a problem hiding this comment.
I actually wrote these comments, because I find that it helps visually break up the test into its constituent parts. Each comment describes the goal of the next few lines.
There was a problem hiding this comment.
I find the opposite, code comments make it more difficult for me to read the code. Code comments often lie, are written in a non-technical language, or are too verbose. Comments should only be used when strictly necessary to understand the code and when the variable name or function name cannot be improved.
In unit tests, code comments are treated a bit more loosely, but the comments in this file do not help me much. Some comments can stay, e.g. $link->close() is nice.
There was a problem hiding this comment.
I see. It may be difficult and require a bit of back-and-forth to understand which comments you feel are useful and which are not. Would you mind taking a stab at making these tests more to your liking? You could also address some of the other nits you pointed out. I'm not precious about the tests.
If not, do you consider this blocking, or would you be open to restyling the tests later on?
| if (!$link) { | ||
| printf("[001] Cannot connect\n"); | ||
| } |
There was a problem hiding this comment.
I feel like since this is a new test, you could just use the exception mode.
| } | ||
|
|
||
| // Sanity check that the init command was executed. | ||
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0]; |
There was a problem hiding this comment.
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0]; | |
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_column(); |
| // Reopen the same persistent connection, which should be reset, but preserve | ||
| // the options from the initial configuration. |
There was a problem hiding this comment.
What does this mean? What do you test here? What does this test have to do with persistent connections? Shouldn't it test that the configuration is cleared?
There was a problem hiding this comment.
No, the connection configuration should be preserved, and any post-connection changes should be reset.
There was a problem hiding this comment.
By "connection configuration" you mean the two initial commands you execute via options? MYSQLI_SET_CHARSET_NAME and MYSQLI_INIT_COMMAND?
There was a problem hiding this comment.
Yep! And any others, if we're missing them.
| mysqli_options($link, MYSQLI_SET_CHARSET_NAME, 'latin1'); | ||
| mysqli_options($link, MYSQLI_INIT_COMMAND, 'SET @init_marker = 42'); |
There was a problem hiding this comment.
Are these only executed when a persistent connection is opened for the first time?
There was a problem hiding this comment.
That is correct, and they are lost when we issue the COM_RESET_CONNECTION command. The restart_psession function ensures we restore them.
| // Pick a charset the connection does not already use, so the change is observable. | ||
| $default = mysqli_character_set_name($link); | ||
|
|
||
| if ($default === 'latin1') { | ||
| printf("[002] Test needs a default charset other than latin1, got %s\n", $default); | ||
| } |
There was a problem hiding this comment.
Where does the "Pick a charset the connection" happen?
There was a problem hiding this comment.
I believe this would be whatever the MySQL default is, it just can't be latin1 for the purposes of this test.
There was a problem hiding this comment.
So the test doesn't pick a charset? This comment is misleading. This means the test fails when MySQL uses latin1 as the default charset. I belive the test should actually select the charset that won't conflict.
There was a problem hiding this comment.
I'm not sure I see how the comment is misleading - the test does pick a charset, it picks latin1, and fails if the server is already set to latin1. Are you suggesting that we dynamically pick a charset that is guaranteed to not be the server default?
There was a problem hiding this comment.
Yes, that's how I understood this comment.
| } | ||
|
|
||
| // Check the session variable to ensure it is no longer present. | ||
| $v = mysqli_query($link, "SELECT @test_var")->fetch_row()[0]; |
There was a problem hiding this comment.
| $v = mysqli_query($link, "SELECT @test_var")->fetch_row()[0]; | |
| $v = mysqli_query($link, "SELECT @test_var")->fetch_column(); |
|
|
||
| if (mysqli_character_set_name($link) !== $server) { | ||
| printf("[008] Client charset %s disagrees with server %s\n", mysqli_character_set_name($link), $server); | ||
| } |
There was a problem hiding this comment.
I don't understand the whole thing with charsets. Does COM_RESET_CONNECTION reset the server and client charsets? What charset do we expect here and why? This should be made clearer with SELECT @@session.character_set_connection = 'latin1' and $mysqli->character_set_name() === 'latin1,
There was a problem hiding this comment.
Yes, COM_RESET_CONNECTION resets the server charset. Your suggestion may make this clearer, however.
| MyG(num_inactive_persistent)--; | ||
| /* reset variables */ | ||
|
|
||
| #ifndef MYSQLI_NO_CHANGE_USER_ON_PCONNECT |
There was a problem hiding this comment.
Are we dropping this configuration? Was this part of the RFC?
There was a problem hiding this comment.
This is a good point; it was not a part of the RFC.
When I originally wrote this, I could not find any users of MYSQLI_NO_CHANGE_USER_ON_PCONNECT, but we could preserve it - it would just be a bit of a misnomer. We could also introduce a new name for it.
There was a problem hiding this comment.
I don't see how this was not part of the RFC.
The RFC specifies that mysqlnd will call COM_RESET_CONNECTION when reusing persistent connections, which is happening in mysqlnd_restart_psession() which is outside of the #ifdef. Since MySQLnd is the only supported driver for MySQLi, this change is just an obvious cleanup removing useless code.
There was a problem hiding this comment.
It doesn't say anything about removal of mysqli_change_user_silent (see my comment below about the side effect it has got and likely introduction of new state leak.
There was a problem hiding this comment.
It doesn't say anything about removal of mysqli_change_user_silent
My understanding is that the change user is just a poor man’s reset and there is no reason to change the user and then reset the connection when just resetting it is sufficient.
Now if the active database is not reset by the COM_RESET_CONNECTION that is a bug in the implementation that @ericnorris should verify and include a test for. But any fix must then be applied to both PDO and MySQLi, not just MySQLi. And ideally it should be fixed in mysqlnd.
| /* Re-execute any MYSQL_INIT_COMMAND commands. */ | ||
| if (ret == PASS) { | ||
| ret = conn->m->execute_init_commands(conn); | ||
| } |
There was a problem hiding this comment.
I am not sure I like this change. This was not like this before IIRC; it's a BC. I guess that makes sense, but considering it's a BC, it would probably be better without it.
There was a problem hiding this comment.
Leaving it out would be a BC break for PDO; users would have persistent connections that no longer had the state of their INIT commands. Current mysqlnd users already have this due to COM_CHANGE_USER, which is arguably a bug, and we'd "fix" it here.
There was a problem hiding this comment.
Isn't the point of this RFC to introduce a BC for PDO? Previously, a persistent connection in PDO would be in the state in which it disconnected, but now it should be back to the state it was just after the connection was established (but before the first command was issued).
I do understand that this is kind of wacky, so perhaps we should split it. Leave it as it was for mysqli and from PDO retrigger the init_commands separately after successful restart.
There was a problem hiding this comment.
I believe the RFC was scoped to introduce a very particular BC break for PDO - that you would be unable to use persistent connections unless you were using a recent version of MySQL. I do not think it gives us liberty to reset configuration specified at connection time.
I'm not sure how I feel about splitting this, as I think it'd be beneficial to have parity between PDO and mysqlnd. It would also make the code more complicated. That said, it's your (and the other maintainer's) call.
There was a problem hiding this comment.
Yes, it's a tough decision. However, what you consider a "fix" isn't necessarily fixing something that's broken.
We have 3 options:
- apply it in mysqlnd - breaks current mysqli code in a subtle way with no easy fix
- apply it only in PDO - small PDO break but we are breaking stuff anyway, so it's probably fine. mysqli keeps working as before
- don't apply it anywhere - breaks PDO more but also is easier to fix for user. mysqli keeps working as before
I am still leaning towards "don't apply it anywhere" as it seems the safest option to me. Consider code that uses "INSERT INTO table" as an INIT command. With this change, PDO will silently add a new row whenever the connection is reset. There is no way to bring back the old behaviour. If we do not reapply the init command on reset, then the code works as it did before. Will they lose the temporary variables/tables etc.? Yes, but then they can move the statement into exec() and all is fixed.
There was a problem hiding this comment.
I'm going to name your options (a), (b), and (c) for clarity. I also am assuming that in option (a) you mean that we could apply it in both mysqlnd and PDO.
As you noted the correct answer depends on the definition of "fix". I believe that anything that does not return the connection to the exact same state as a fresh connection is a bug, and anything that introduces this behavior where it didn't exist before is a backwards compatibility break. I also believe that @TimWolla agrees with this, but he can correct me if I'm off the mark. The tests reflect this: if you specify a charset in the DSN, it should persist; If you specify an INIT command, it should execute.
Reflecting on the current state of the world under the above definition:
- PDO preserves both the DSN character set and INIT command because it doesn't do any sort of resetting at all. It therefore also preserves session variables, transactions, and other related intermediate state, which is considered a bug according to the above definition. The RFC gives us license to reset this intermediate state, but does not give us license to reset the DSN character set nor state of the INIT command. Additionally, while it is technically possible to have an application that relies on the intermediate state, it's purely hypothetical: you would need to have an application that (1) sets up a persistent connection, and (2) somehow does not rely on the intermediate state for fresh connections as it wouldn't be there, and (3) applies intermediate state but only for fresh connections, and (4) somehow relies on the intermediate state only for persistent connections. I would argue that this is not a BC break as no reasonable code would do this, but even so, again, the RFC gives us license to make the change. From now on I will not refer to this as a BC break.
- mysqlnd preserves the DSN character set but does not preserve the INIT command. This is a bug according to the above definition. I also don't think that re-executing the INIT command counts as a BC break - I don't see how calling
INSERT INTO tableon reset would actually break anything, realistically. The option is documented by both PDO and MySQL as being "automatically re-executed if reconnection occurs", and so code using it would need to expect that since a reconnection could happen at any time, it's liable to execute multiple times anyways.
Exploring your options based on the above:
- (a) would preserve the DSN character set and INIT command (thus, BC) for PDO and would fix a bug (not preserving the INIT command) in mysqlnd, and is therefore permissible and desirable.
- (b) would preserve the DSN character set and INIT command (thus, BC) for PDO, and would leave mysqlnd broken, and is therefore permissible but undesirable from a parity, code complexity, and bug standpoint.
- (c) would preserve the DSN character set but reset the state of an INIT command (thus, would break BC) for PDO, and would leave myslqnd broken, and is therefore impermissible.
There was a problem hiding this comment.
For option (a), I meant only in mysqlnd. No need to apply it twice in PDO and mysqlnd.
I was not aware that the documentation states "Will automatically be re-executed when reconnecting.". I don't know why it says that, but I guess we can use that to allow us to make a decision. It also says that for mysqli, which makes me think it was a copy-paste error in the PDO documentation. The mysqli extension doesn't have a reconnection feature anymore, but when it did, I assume it re-sent the init commands. Resetting a persistent connection is not the same as reconnection, but I guess we can avail of this historical artefact to justify the BC.
Ok then, keep it as you are proposing now. Let's go with option (a) and apply it in mysqlnd. However, this requires an UPGRADING entry explaining this breaking change for mysqli.
There was a problem hiding this comment.
I don't think mysqli should be impacted by this. This wasn't part of the RFC so we should do as suggested in #21857 (comment)
There was a problem hiding this comment.
I also believe that TimWolla agrees with this, but he can correct me if I'm off the mark.
That is an accurate representation of my opinion on the matter.
| /* {{{ mysqlnd_conn_data::restart_psession */ | ||
| static void | ||
| static enum_func_status | ||
| MYSQLND_METHOD(mysqlnd_conn_data, restart_psession)(MYSQLND_CONN_DATA * conn) |
There was a problem hiding this comment.
From what I understand, this function didn't do the reconnection before. By you adding it, you are breaking ABI. Extensions using mysqlnd (other than mysqli and pdo_mysql) will now silently get buggy behaviour.
There was a problem hiding this comment.
I'm not sure what you mean by silently buggy behavior, but do you have a suggestion for what we should do otherwise?
There was a problem hiding this comment.
Don't modify this method. Add a new method. We need to keep restart_psession exactly as it was so as not to break existing extensions.
There was a problem hiding this comment.
How would this break existing extensions? If it does break them, why is it problematic to break existing extensions? Shouldn't restart_psession do everything that is necessary to restart a persistent session?
I don't know that I feel strongly here, to be clear, but I don't understand why restart_psession shouldn't do exactly that. If you want to add a new method, what do we call it? Teasingly - actually_restart_psession / restart_psession_correctly / restart_psession_v2?
There was a problem hiding this comment.
Consider someone has designed PDO_Improved2 and they use it in their closed-source project. Their extension uses mysqlnd calls just like PDO does. It calls restart_psession with the assumption that it does what it always did: pretty much nothing. If you change the signature and the assumption, you could cause their extension to no longer function. PHP promises no ABI breaks in minor versions, so we'd be breaking this promise.
There is technically very little chance that this will cause problems for someone, but we don't know. Big companies keep secrets.
As for the new name, reset_connection should be fine.
There was a problem hiding this comment.
I just don't like the idea of introducing cruft here - any reasonable person looking at restart_psession would think "this should reset a persistent session" and then they'd discover, "oh, no, it doesn't really, you actually need to call reset_connection."
That said, like I mentioned I don't feel strongly here. If you don't object, I will leave a TODO mentioning that we should merge reset_connection into restart_psession, and delete reset_connection, in PHP 9.0.
There was a problem hiding this comment.
I agree with you, but that's not the point. mysqlnd is a public API, and we cannot just change it however we want.
I have no objections to removing restart_psession in PHP 9.0, but I think the name reset_connection is more fitting considering the name of this PR.
There was a problem hiding this comment.
PHP promises no ABI breaks in minor versions, so we'd be breaking this promise.
This is false. There are much less guarantees regarding the internal API and breaking changes are commonly happening (just have a look at UPGRADING.INTERNALS). See also https://github.com/php/policies/blob/main/release-process.rst#minor-version-number.
The RFC also was pretty clear in that it would adjust mysqlnd:
This RFC proposes that we use COM_RESET_CONNECTION in PDO and mysqlnd when reusing persistent connections.
There was a problem hiding this comment.
You're right. I'm sorry. The ABI may be broken, but it shouldn't be. I still think adding a new function with a better name is better than reusing the old one.
I think the guidance has recently been changed. Previously, it said that only the internal ABI can be broken.
| $pass = PDO_MYSQL_TEST_PASS; | ||
| $opts = [ | ||
| PDO::ATTR_PERSISTENT => true, | ||
| PDO::ATTR_ERRMODE => PDO::ERRMODE_SILENT, |
There was a problem hiding this comment.
Why are we using PDO::ERRMODE_SILENT?
There was a problem hiding this comment.
Ah, this may have been a copy-paste error from the _reset test.
| $user = PDO_MYSQL_TEST_USER; | ||
| $pass = PDO_MYSQL_TEST_PASS; |
There was a problem hiding this comment.
Why did you add mutability if you are not mutating it?
There was a problem hiding this comment.
I'll remove these!
|
|
||
| // Establish a persistent handle with the above options. | ||
| $db1 = new PDO($dsn, $user, $pass, $opts); | ||
| $con1 = $db1->query('SELECT CONNECTION_ID()')->fetchColumn(); |
There was a problem hiding this comment.
mysqli tests should also use SELECT CONNECTION_ID()
| #ifdef PDO_USE_MYSQLND | ||
| pdo_mysql_reset_connection, | ||
| #else | ||
| NULL, /* reset_connection */ |
There was a problem hiding this comment.
So no equivalent on libmysqlclient?
There was a problem hiding this comment.
I think mysql_reset_connection would work there, it would just take more work to implement. I kinda thought that libmysqlclient was less supported / not really supposed to be used, honestly. Is it worth supporting here?
There was a problem hiding this comment.
PDO fully and invisibly supports both. There is nothing you can do with mysqlnd that you can't do with libmysql as of now. I think if we are making such a big change to PDO then we should keep the parity.
| mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx"); | ||
| mysqli_close($link); |
There was a problem hiding this comment.
Should be in a CLEANUP section.
|
Thanks for the review @kamil-tekiela. It's a little hard to respond to the volume of threads, but I will try to go through them all. One thing that I think is worth clarifying here: this PR introduces |
| --TEST-- | ||
| mysqli: reusing a persistent connection re-applies its connection config after reset | ||
| --EXTENSIONS-- | ||
| mysqli | ||
| --SKIPIF-- | ||
| <?php | ||
| require_once 'skipifconnectfailure.inc'; | ||
| ?> | ||
| --FILE-- | ||
| <?php | ||
| // COM_RESET_CONNECTION reverts the session to server defaults; mysqlnd should | ||
| // then re-apply the connection config, so a reused connection stays configured. | ||
|
|
||
| require_once 'connect.inc'; | ||
|
|
||
| $host = 'p:' . $host; | ||
|
|
||
| // Open a persistent connection carrying both a charset and an init command. | ||
| function connect_configured($host, $user, $passwd, $db, $port, $socket) { | ||
| $link = mysqli_init(); | ||
| mysqli_options($link, MYSQLI_SET_CHARSET_NAME, 'latin1'); | ||
| mysqli_options($link, MYSQLI_INIT_COMMAND, 'SET @init_marker = 42'); | ||
|
|
||
| if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) { | ||
| return false; | ||
| } | ||
|
|
||
| return $link; | ||
| } | ||
|
|
||
| // Establish a persistent handle with the above configuration. | ||
| $link = connect_configured($host, $user, $passwd, $db, $port, $socket); | ||
|
|
||
| if (!$link) { | ||
| printf("[001] Cannot connect\n"); | ||
| } | ||
|
|
||
| $thread_id = mysqli_thread_id($link); | ||
|
|
||
| // Sanity check that the charset was configured. | ||
| if (mysqli_character_set_name($link) !== 'latin1') { | ||
| printf("[002] Charset should be latin1 at connect, got %s\n", mysqli_character_set_name($link)); | ||
| } | ||
|
|
||
| // Sanity check that the init command was executed. | ||
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0]; | ||
|
|
||
| if ($marker != 42) { | ||
| printf("[003] Init command should have set @init_marker at connect, got %s\n", var_export($marker, true)); | ||
| } | ||
|
|
||
| // Drift both values away from the configuration, so the checks after reuse prove | ||
| // the config was re-applied by the reset rather than merely carried over. | ||
| mysqli_set_charset($link, 'ascii'); | ||
| mysqli_query($link, "SET @init_marker = 99"); | ||
|
|
||
| // Return the connection to the pool. | ||
| mysqli_close($link); | ||
|
|
||
| // Reopen the same persistent connection, which should be reset, but preserve | ||
| // the options from the initial configuration. | ||
| $link = connect_configured($host, $user, $passwd, $db, $port, $socket); | ||
|
|
||
| // Compare the thread IDs to ensure the connection was reused. | ||
| if (mysqli_thread_id($link) !== $thread_id) { | ||
| printf("[004] Expected the pooled connection to be reused\n"); | ||
| } | ||
|
|
||
| // Check the charset to ensure the configured one is back in effect. | ||
| if (mysqli_character_set_name($link) !== 'latin1') { | ||
| printf("[005] Charset should be re-applied to latin1, got %s\n", mysqli_character_set_name($link)); | ||
| } | ||
|
|
||
| // Check the server-side charset to ensure it agrees with the client. | ||
| $server = mysqli_query($link, "SELECT @@session.character_set_connection")->fetch_row()[0]; | ||
|
|
||
| if ($server !== 'latin1') { | ||
| printf("[006] Server charset should be latin1, got %s\n", $server); | ||
| } | ||
|
|
||
| // Check the session variable to ensure the init command was re-executed. | ||
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0]; | ||
|
|
||
| if ($marker != 42) { | ||
| printf("[007] Init command should be re-run after reuse, got %s\n", var_export($marker, true)); | ||
| } | ||
|
|
||
| mysqli_close($link); | ||
|
|
||
| echo "done!"; | ||
| ?> | ||
| --EXPECT-- | ||
| done! |
There was a problem hiding this comment.
| --TEST-- | |
| mysqli: reusing a persistent connection re-applies its connection config after reset | |
| --EXTENSIONS-- | |
| mysqli | |
| --SKIPIF-- | |
| <?php | |
| require_once 'skipifconnectfailure.inc'; | |
| ?> | |
| --FILE-- | |
| <?php | |
| // COM_RESET_CONNECTION reverts the session to server defaults; mysqlnd should | |
| // then re-apply the connection config, so a reused connection stays configured. | |
| require_once 'connect.inc'; | |
| $host = 'p:' . $host; | |
| // Open a persistent connection carrying both a charset and an init command. | |
| function connect_configured($host, $user, $passwd, $db, $port, $socket) { | |
| $link = mysqli_init(); | |
| mysqli_options($link, MYSQLI_SET_CHARSET_NAME, 'latin1'); | |
| mysqli_options($link, MYSQLI_INIT_COMMAND, 'SET @init_marker = 42'); | |
| if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) { | |
| return false; | |
| } | |
| return $link; | |
| } | |
| // Establish a persistent handle with the above configuration. | |
| $link = connect_configured($host, $user, $passwd, $db, $port, $socket); | |
| if (!$link) { | |
| printf("[001] Cannot connect\n"); | |
| } | |
| $thread_id = mysqli_thread_id($link); | |
| // Sanity check that the charset was configured. | |
| if (mysqli_character_set_name($link) !== 'latin1') { | |
| printf("[002] Charset should be latin1 at connect, got %s\n", mysqli_character_set_name($link)); | |
| } | |
| // Sanity check that the init command was executed. | |
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0]; | |
| if ($marker != 42) { | |
| printf("[003] Init command should have set @init_marker at connect, got %s\n", var_export($marker, true)); | |
| } | |
| // Drift both values away from the configuration, so the checks after reuse prove | |
| // the config was re-applied by the reset rather than merely carried over. | |
| mysqli_set_charset($link, 'ascii'); | |
| mysqli_query($link, "SET @init_marker = 99"); | |
| // Return the connection to the pool. | |
| mysqli_close($link); | |
| // Reopen the same persistent connection, which should be reset, but preserve | |
| // the options from the initial configuration. | |
| $link = connect_configured($host, $user, $passwd, $db, $port, $socket); | |
| // Compare the thread IDs to ensure the connection was reused. | |
| if (mysqli_thread_id($link) !== $thread_id) { | |
| printf("[004] Expected the pooled connection to be reused\n"); | |
| } | |
| // Check the charset to ensure the configured one is back in effect. | |
| if (mysqli_character_set_name($link) !== 'latin1') { | |
| printf("[005] Charset should be re-applied to latin1, got %s\n", mysqli_character_set_name($link)); | |
| } | |
| // Check the server-side charset to ensure it agrees with the client. | |
| $server = mysqli_query($link, "SELECT @@session.character_set_connection")->fetch_row()[0]; | |
| if ($server !== 'latin1') { | |
| printf("[006] Server charset should be latin1, got %s\n", $server); | |
| } | |
| // Check the session variable to ensure the init command was re-executed. | |
| $marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0]; | |
| if ($marker != 42) { | |
| printf("[007] Init command should be re-run after reuse, got %s\n", var_export($marker, true)); | |
| } | |
| mysqli_close($link); | |
| echo "done!"; | |
| ?> | |
| --EXPECT-- | |
| done! | |
| --TEST-- | |
| mysqli: reusing a persistent connection should reapply MYSQLI_SET_CHARSET_NAME and MYSQLI_INIT_COMMAND options | |
| --EXTENSIONS-- | |
| mysqli | |
| --SKIPIF-- | |
| <?php | |
| require_once 'skipifconnectfailure.inc'; | |
| ?> | |
| --FILE-- | |
| <?php | |
| require_once 'connect.inc'; | |
| mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); | |
| function connect_with_options($host, $user, $passwd, $db, $port, $socket): mysqli { | |
| $link = new mysqli(); | |
| $link->options(MYSQLI_SET_CHARSET_NAME, 'latin1'); | |
| $link->options(MYSQLI_INIT_COMMAND, 'SET @init_marker = 42'); | |
| $link->connect('p:' . $host, $user, $passwd, $db, $port, $socket); | |
| return $link; | |
| } | |
| $link = connect_with_options($host, $user, $passwd, $db, $port, $socket); | |
| $connection_id = $link->query("SELECT CONNECTION_ID()")->fetch_column(); | |
| if ($link->character_set_name() !== 'latin1') { | |
| printf("[001] Charset should be latin1 at connect, got %s\n", $link->character_set_name()); | |
| } | |
| $marker = $link->query("SELECT @init_marker")->fetch_column(); | |
| if ($marker != 42) { | |
| printf("[002] Init command should have set @init_marker at connect, got %s\n", var_export($marker, true)); | |
| } | |
| // Drift both values away from the configuration, so the checks after reuse prove | |
| // the config was reapplied by the reset rather than merely carried over. | |
| $link->set_charset('ascii'); | |
| $link->query("SET @init_marker = 99"); | |
| // Return the connection to the pool. | |
| $link->close(); | |
| // Reopen the same persistent connection, which should be reset but have the same options reapplied. | |
| $link = connect_with_options($host, $user, $passwd, $db, $port, $socket); | |
| if ($link->query("SELECT CONNECTION_ID()")->fetch_column() !== $connection_id) { | |
| printf("[003] Expected the pooled connection to be reused\n"); | |
| } | |
| if ($link->character_set_name() !== 'latin1') { | |
| printf("[004] Charset should be reapplied to latin1, got %s\n", $link->character_set_name()); | |
| } | |
| // Check the server-side charset to ensure it agrees with the client. | |
| $server = $link->query("SELECT @@session.character_set_connection")->fetch_column(); | |
| if ($server !== 'latin1') { | |
| printf("[005] Server charset should be latin1, got %s\n", $server); | |
| } | |
| $marker = $link->query("SELECT @init_marker")->fetch_column(); | |
| if ($marker != 42) { | |
| printf("[006] Init command should be re-run after reuse, got %s\n", var_export($marker, true)); | |
| } | |
| echo "done!"; | |
| ?> | |
| --EXPECT-- | |
| done! |
| --TEST-- | ||
| mysqli: reusing a persistent connection resets its session state (COM_RESET_CONNECTION) | ||
| --EXTENSIONS-- | ||
| mysqli | ||
| --SKIPIF-- | ||
| <?php | ||
| require_once 'skipifconnectfailure.inc'; | ||
| ?> | ||
| --FILE-- | ||
| <?php | ||
| // Reusing a pooled persistent connection sends COM_RESET_CONNECTION, which must | ||
| // wipe all existing session state. | ||
|
|
||
| require_once 'connect.inc'; | ||
|
|
||
| $host = 'p:' . $host; | ||
|
|
||
| // Establish a persistent handle with some session state. | ||
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | ||
|
|
||
| if (!$link) { | ||
| printf("[001] Cannot connect\n"); | ||
| } | ||
|
|
||
| $thread_id = mysqli_thread_id($link); | ||
|
|
||
| // Pick a charset the connection does not already use, so the change is observable. | ||
| $default = mysqli_character_set_name($link); | ||
|
|
||
| if ($default === 'latin1') { | ||
| printf("[002] Test needs a default charset other than latin1, got %s\n", $default); | ||
| } | ||
|
|
||
| mysqli_query($link, "SET @test_var = 42"); | ||
| mysqli_query($link, "CREATE TEMPORARY TABLE test_reset_tmp (id INT)"); | ||
|
|
||
| mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx"); | ||
| mysqli_query($link, "CREATE TABLE test_reset_trx (id INT) ENGINE=InnoDB"); | ||
|
|
||
| mysqli_begin_transaction($link); | ||
| mysqli_query($link, "INSERT INTO test_reset_trx VALUES (1)"); | ||
|
|
||
| // Change the charset after connecting, which is not part of the connection config. | ||
| mysqli_set_charset($link, 'latin1'); | ||
|
|
||
| // Return the connection to the pool. | ||
| mysqli_close($link); | ||
|
|
||
| // Reopen the same persistent connection, which should be reset. | ||
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | ||
|
|
||
| // Compare the thread IDs to ensure the connection was reused. | ||
| if (mysqli_thread_id($link) !== $thread_id) { | ||
| printf("[003] Expected the pooled connection to be reused\n"); | ||
| } | ||
|
|
||
| // Check the session variable to ensure it is no longer present. | ||
| $v = mysqli_query($link, "SELECT @test_var")->fetch_row()[0]; | ||
|
|
||
| if ($v !== null) { | ||
| printf("[004] User variable should be reset, got %s\n", var_export($v, true)); | ||
| } | ||
|
|
||
| // Check the temporary table to ensure it is no longer present. | ||
| if (@mysqli_query($link, "SELECT 1 FROM test_reset_tmp") !== false) { | ||
| printf("[005] Temporary table should not exist after reset\n"); | ||
| } | ||
|
|
||
| // Check to see we are no longer in a transaction. | ||
| $rows = mysqli_query($link, "SELECT COUNT(*) FROM test_reset_trx")->fetch_row()[0]; | ||
|
|
||
| if ($rows != 0) { | ||
| printf("[006] Transaction should have been rolled back, found %d row(s)\n", $rows); | ||
| } | ||
|
|
||
| // A charset set after connecting is not part of the config, so it is not restored. | ||
| if (mysqli_character_set_name($link) !== $default) { | ||
| printf("[007] Charset should be reset to %s, got %s\n", $default, mysqli_character_set_name($link)); | ||
| } | ||
|
|
||
| // mysqlnd's cached charset (used for escaping) must still agree with the server. | ||
| $server = mysqli_query($link, "SELECT @@session.character_set_connection")->fetch_row()[0]; | ||
|
|
||
| if (mysqli_character_set_name($link) !== $server) { | ||
| printf("[008] Client charset %s disagrees with server %s\n", mysqli_character_set_name($link), $server); | ||
| } | ||
|
|
||
| // Clean up the non-temporary table. | ||
| mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx"); | ||
| mysqli_close($link); | ||
|
|
||
| echo "done!"; | ||
| ?> | ||
| --EXPECT-- | ||
| done! |
There was a problem hiding this comment.
| --TEST-- | |
| mysqli: reusing a persistent connection resets its session state (COM_RESET_CONNECTION) | |
| --EXTENSIONS-- | |
| mysqli | |
| --SKIPIF-- | |
| <?php | |
| require_once 'skipifconnectfailure.inc'; | |
| ?> | |
| --FILE-- | |
| <?php | |
| // Reusing a pooled persistent connection sends COM_RESET_CONNECTION, which must | |
| // wipe all existing session state. | |
| require_once 'connect.inc'; | |
| $host = 'p:' . $host; | |
| // Establish a persistent handle with some session state. | |
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | |
| if (!$link) { | |
| printf("[001] Cannot connect\n"); | |
| } | |
| $thread_id = mysqli_thread_id($link); | |
| // Pick a charset the connection does not already use, so the change is observable. | |
| $default = mysqli_character_set_name($link); | |
| if ($default === 'latin1') { | |
| printf("[002] Test needs a default charset other than latin1, got %s\n", $default); | |
| } | |
| mysqli_query($link, "SET @test_var = 42"); | |
| mysqli_query($link, "CREATE TEMPORARY TABLE test_reset_tmp (id INT)"); | |
| mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx"); | |
| mysqli_query($link, "CREATE TABLE test_reset_trx (id INT) ENGINE=InnoDB"); | |
| mysqli_begin_transaction($link); | |
| mysqli_query($link, "INSERT INTO test_reset_trx VALUES (1)"); | |
| // Change the charset after connecting, which is not part of the connection config. | |
| mysqli_set_charset($link, 'latin1'); | |
| // Return the connection to the pool. | |
| mysqli_close($link); | |
| // Reopen the same persistent connection, which should be reset. | |
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | |
| // Compare the thread IDs to ensure the connection was reused. | |
| if (mysqli_thread_id($link) !== $thread_id) { | |
| printf("[003] Expected the pooled connection to be reused\n"); | |
| } | |
| // Check the session variable to ensure it is no longer present. | |
| $v = mysqli_query($link, "SELECT @test_var")->fetch_row()[0]; | |
| if ($v !== null) { | |
| printf("[004] User variable should be reset, got %s\n", var_export($v, true)); | |
| } | |
| // Check the temporary table to ensure it is no longer present. | |
| if (@mysqli_query($link, "SELECT 1 FROM test_reset_tmp") !== false) { | |
| printf("[005] Temporary table should not exist after reset\n"); | |
| } | |
| // Check to see we are no longer in a transaction. | |
| $rows = mysqli_query($link, "SELECT COUNT(*) FROM test_reset_trx")->fetch_row()[0]; | |
| if ($rows != 0) { | |
| printf("[006] Transaction should have been rolled back, found %d row(s)\n", $rows); | |
| } | |
| // A charset set after connecting is not part of the config, so it is not restored. | |
| if (mysqli_character_set_name($link) !== $default) { | |
| printf("[007] Charset should be reset to %s, got %s\n", $default, mysqli_character_set_name($link)); | |
| } | |
| // mysqlnd's cached charset (used for escaping) must still agree with the server. | |
| $server = mysqli_query($link, "SELECT @@session.character_set_connection")->fetch_row()[0]; | |
| if (mysqli_character_set_name($link) !== $server) { | |
| printf("[008] Client charset %s disagrees with server %s\n", mysqli_character_set_name($link), $server); | |
| } | |
| // Clean up the non-temporary table. | |
| mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx"); | |
| mysqli_close($link); | |
| echo "done!"; | |
| ?> | |
| --EXPECT-- | |
| done! | |
| --TEST-- | |
| mysqli: reusing a persistent connection should reset its session state (COM_RESET_CONNECTION) | |
| --EXTENSIONS-- | |
| mysqli | |
| --SKIPIF-- | |
| <?php | |
| require_once 'skipifconnectfailure.inc'; | |
| ?> | |
| --FILE-- | |
| <?php | |
| require_once 'connect.inc'; | |
| mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); | |
| $host = 'p:' . $host; | |
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | |
| $connection_id = $link->query("SELECT CONNECTION_ID()")->fetch_column(); | |
| // Pick a charset the connection does not already use, so the change is observable. | |
| $default_charset = $link->character_set_name(); | |
| $charset = $default_charset === 'latin1' ? 'utf8mb4' : 'latin1'; | |
| $link->set_charset($charset); | |
| $link->query("SET @test_var = 42"); | |
| $link->query("CREATE TEMPORARY TABLE test_reset_tmp (id INT)"); | |
| $link->query("DROP TABLE IF EXISTS test_reset_trx"); | |
| $link->query("CREATE TABLE test_reset_trx (id INT) ENGINE=InnoDB"); | |
| $link->begin_transaction(); | |
| $link->query("INSERT INTO test_reset_trx VALUES (1)"); | |
| // Return the connection to the pool. | |
| $link->close(); | |
| // Reopen the same persistent connection, which should be reset. | |
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | |
| if ($link->query("SELECT CONNECTION_ID()")->fetch_column() !== $connection_id) { | |
| printf("[003] Expected the persistent connection to be reused\n"); | |
| } | |
| $v = $link->query("SELECT @test_var")->fetch_column(); | |
| if ($v !== null) { | |
| printf("[004] User variable should be reset, got %s\n", var_export($v, true)); | |
| } | |
| try { | |
| $link->query("SELECT * FROM test_reset_tmp"); | |
| printf("[005] Temporary table should have been dropped\n"); | |
| } catch (mysqli_sql_exception $e) { | |
| if ($e->getCode() !== 1146) { // ER_NO_SUCH_TABLE | |
| throw $e; | |
| } | |
| } | |
| $rows = $link->query("SELECT COUNT(*) FROM test_reset_trx")->fetch_column(); | |
| if ($rows !== 0) { | |
| printf("[006] Transaction should have been rolled back, found %d row(s)\n", $rows); | |
| } | |
| // A charset set after connecting is not part of the config, so it is not restored. | |
| if ($link->character_set_name() !== $default_charset) { | |
| printf("[007] Charset should be reset to %s, got %s\n", $default_charset, $link->character_set_name()); | |
| } | |
| // mysqlnd's charset must still agree with the server. | |
| $server = $link->query("SELECT @@session.character_set_connection")->fetch_column(); | |
| if ($link->character_set_name() !== $server) { | |
| printf("[008] Client charset %s disagrees with server %s\n", $link->character_set_name(), $server); | |
| } | |
| echo "done!"; | |
| ?> | |
| --CLEAN-- | |
| <?php | |
| require_once 'connect.inc'; | |
| mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); | |
| $link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket); | |
| $link->query("DROP TABLE IF EXISTS test_reset_trx"); | |
| ?> | |
| --EXPECT-- | |
| done! |
| /* reset variables */ | ||
|
|
||
| #ifndef MYSQLI_NO_CHANGE_USER_ON_PCONNECT | ||
| if (!mysqli_change_user_silent(mysql->mysql, username, passwd, dbname, passwd_len)) { |
There was a problem hiding this comment.
So this actually introduce new state leak. Before this changes the default database was restored but after this, it is kept because COM_RESET_CONNECTION does not seem to touch it.
There was a problem hiding this comment.
This looks actually like a significant regression because I think the impact is following:
// Request A
$db = mysqli_connect('p:localhost', $user, $pass, 'app');
$db->select_db('archive'); // or $db->query('USE archive');
// request ends, connection goes back to the pool
// Request B
$db = mysqli_connect('p:localhost', $user, $pass, 'app'); // same hash key => reuse
$db->query('SELECT * FROM users'); // runs against archive.users, not app.usersIf this is really the case, then this PR definitely needs a proper review so will need to allocate more time to go through it properly and think about other consequences to make sure there aren't other issues.
7 participants
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.