Hi internals,
There's been some discussion over how to deal with persistent
connections in PDO. I know this is a common problem users deal with;
sometimes the connection has been put into a bad state (i.e. search
paths or some other connection-level state being set by user code),
or the PDO liveness check may not be perfect for all drivers (i.e.
for PDO_ODBC, it depends heavily on the connection dead attribute
implementation in the driver). I've at least acknowledged this as a
general problem for persistent resources/objects before (GH-15749).
I've seen users cope with bad persistent connections by killing the
PHP process running into it, which is obviously not ideal.
There's been some proposals to fix this; mostly by adding a close
method to PDO (GH-19933/GH-19214), but this complicates PDO by making
closed connection objects a thing. I have a draft PR (GH-21007) that
simply provides an attribute for throwing away the current persistent
connection and getting a new one; this allows userland code to
implement their own reconnect and recovery behaviours. However, I'm
still not very sure it's the best approach, as said recovery code is
awkward code that users need to implement for themselves. I've thought
about some alternatives (i.e. allow hooking liveness checks to
supplement with user code), but I haven't implemented those.
I'd appreciate some thoughts or proposals on the manner, since this
is a papercut that I've seen users deal with.
Regards,
Calvin
GH issues/PRs mentioned:
opened 05:13PM - 04 Sep 24 UTC
Feature
Status: Verified
### Description
PHP has been moving towards using objects instead of resources.… One change as a result of this is that the resource the object represents gets closed on the destructor, instead of calling `extension_providing_resource_close` methods. Normally, this is OK, as you can just remove all references to an object and have it clean up. However, with persistent resources, the extension holds a reference onto the object that it provides to you, preventing you from being able to destroy the object. This use case might exist for i.e. cleaning up a persistent database connection that has gotten itself into a wedged state that the extension can't handle (i.e. ODBC liveness checks aren't perfect), or closing something to finalize something done over multiple requests.
The easiest option may be to add a close method to the object, similar to what i.e. SQLite3 does. Some extensions like curl already have close methods, but they might have been converted to nops in the resource to object conversion. However, this adds issues with having an object that represents i.e. a connection in a closed state. (However, since one of the cases is say, closing a database connection, we already might have to deal with live objects that represent a resource that's been closed.)
Another option might be to expose a list of currently used persistent resources. `get_resources` can be abused for this, but contains massive footguns and only works with legacy resources. Perhaps this can be done for persistent objects in general in a safer manner, or have a persistent object list provided by the extension. Persistent object issues can be hard to debug, so this might be generally useful.
See: https://github.com/php/php-src/pull/15603#issuecomment-2329536824 - there might be other conversations on this
PHP-8.3 ← rposky:PHP-8.3
opened 04:48PM - 22 Jul 25 UTC
The principal target of this PR is to introduce `PDO::disconnect()` to disconnec… t from the database as an alternative to PDO's destructor, which feature is related to bug [#40681](https://bugs.php.net/bug.php?id=40681) and requests [#62065](https://bugs.php.net/bug.php?id=62065) and [#67473](https://bugs.php.net/bug.php?id=67473). Further and related efforts have been made herein to modify the destruct behavior of persistent PDO db connections. The rollback invocation questioned with bug [#63343](https://bugs.php.net/bug.php?id=63343) is postponed until time of disconnect as opposed to PDO object destruct, given the potential to affect other PDO instances. Additionally, PDO object integrity is maintained w.r.t. it's inner db handle in the case that a liveness check fails upon other PDO construct and a new persistent resource is registered.
Current recommendations in PDO application integration avoid the need for an explicit disconnect routine and workaround the above described issues through prescribed use of a single PDO object instance for any given database connection, and which database connection is terminated upon either PDO object destruct or, in the case of persistent connections, at the end of script execution. While the use of a singleton object for database connection handling within applications is a common implementation pattern, the tight integration of database disconnect behavior with PHP's object free hook has imposed a need to further manage PDO object references within an application in order to reliably replace and/or shutdown a db connection instance, an effort which scales with application complexity. This reality is further complicated by the PDO statement object, which also holds a reference to a PDO object and thus must also be managed from a reference standpoint, as well as by PHP itself, which governs the time at which an object free hook is invoked, subject to influence by bug or design. It is also worth noting that in the current form, it is not possible to use the PDO object to determine the state of database connection closure, whether normal or errant.
The proposed changes add application integration options without majorly modifying current behavior, but the following behavior changes are present.
- When a db connection has been closed through call to `PDO::disconnect()`, subsequent invocations against the same db handle by any surviving PDO objects will throw a novel `PDOException` with the message "connection is closed".
- The `_pdo_object_t::is_closed` struct member is put to use, presently not referenced within the source. Other driver implementations which have used the struct member in unintended ways may suffer.
- The `_pdo_object_t::refcount` struct member accurately reflects a count of PDO objects referring to it, regardless of persistent connection state.
- Both `PDO::errorInfo()` and `PDO::errorCode()` may now be called on an uninitialized PDO object, i.e. one without a defined `driver` member. This change is being done to permit use of both functions after `PDO::disconnect()` is called, and as a result of modifying the `PDO_CONSTRUCT_CHECK` macro to impose check on `is_closed`.
- Do not rollback a persistent db connection when a PDO object is freed.
- Do not free memory for a persistent db connection when its resource is destructed if still referenced by PDO objects. This can occur when constructing a new PDO object and the current persistent db connection fails a liveness check.
I trust you will inform me as this submission has departed from the contribution guidelines or coding standards in any way that I may have missed, and welcome your feedback.
master ← rposky:pdo-disconnect
opened 02:24PM - 23 Sep 25 UTC
An RFC accompanying this PR will soon be created, and I will update this descrip… tion with a link once it is so.
This PR was initially opened as #19214, but which targeted PHP 8.3, and so is re-created here now targeting PHP 8.6.
master ← NattyNarwhal:pdo-pconnect-reconnect
opened 04:07PM - 22 Jan 26 UTC
The persistent connection may be in a wedged or otherwise undesirable state that… the connection liveness check has failed to deal with. (For instance, the ODBC connection dead attribute that PDO_ODBC uses to check isn't perfectly handled by all drivers, or if a persistent connection has bad persistent state, it's easier to just reconnect if so.) If so, provide userland code a way to get rid of a persistent connection and create a new one in its place.
This is prompted by the discussion in GH-15749 and GH-19933. I think adding a notion of disconnected PDO objects is probably unwise, so this provides an alternative that requires very minimal changes in PDO. I'm not entirely pleased with this approach, but perhaps there is a better one. I don't intend to get this merged, without a discussion anyways - I'll bring that up on internals.