Make PDO connection construction single-shot

[iliaal]

Constructing a PDO over an already constructed handle re-runs connection setup and, for a persistent connection, frees the pemalloc'd dbh with efree(), corrupting the heap. The same sink is also reachable by reentering __construct() from a uri: DSN stream wrapper or a failed connect()'s destructor, and by retrying a persistent construct that failed after swapping in the handle. This makes construction single-shot: an is_constructing flag (an ABI-neutral bit taken from _reserved_flags, sizeof(pdo_dbh_t) unchanged) is set as soon as the handle exists for both __construct() and connect(), and construction is rejected once the driver is attached or one is already in progress. Behavior change: a construct that fails now leaves the handle unusable instead of allowing a retry.

$pdo = new PDO('sqlite::memory:', null, null, [PDO::ATTR_PERSISTENT => true]);
$pdo->__construct('sqlite::memory:', null, null, [PDO::ATTR_PERSISTENT => true]); // heap corruption before this change

[kamil-tekiela]

Why disallow it? Why not just fix the heap corruption?

[iliaal]

I can't think of a legitimate use for re-running __construct() on a live handle, and the heap corruption is just the loudest symptom of re-running one-time setup, not the whole problem. Two of the reachable cases are reentry during construction, a uri: DSN's stream wrapper and a failed persistent connect's __destruct() both re-entering __construct() on the still-constructing object, where reconstruction has no coherent meaning and needs rejecting regardless. Single-shot matches precedent: the SPL directory iterators already throw "already initialized" on a second __construct().

[kamil-tekiela]

I looked at this PR again, and maybe you are right, but I still don't quite follow your fix. Wouldn't something like this work better?

From 917882481c0b5f9d18cd843ef29e209bc090cd3a Mon Sep 17 00:00:00 2001
From: Kamil Tekiela <tekiela246@gmail.com>
Date: Fri, 24 Jul 2026 14:20:16 +0100
Subject: [PATCH] Update pdo_dbh.c

---
 ext/pdo/pdo_dbh.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c
index dcd9f7b126d..5c0113f8860 100644
--- a/ext/pdo/pdo_dbh.c
+++ b/ext/pdo/pdo_dbh.c
@@ -384,6 +384,11 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 		dbh = php_pdo_dbh_fetch_inner(current_object);
 	}
 
+	if (dbh->driver) {
+		zend_throw_error(NULL, "%s object is already constructed", ZSTR_VAL(called_scope->name));
+		RETURN_THROWS();
+	}
+
 	/* is this supposed to be a persistent connection ? */
 	if (options) {
 		zend_string *hash_key = NULL;
@@ -440,7 +445,7 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 		}
 
 		if (pdbh) {
-			efree(dbh);
+			pefree(dbh, dbh->is_persistent);
 
 			pdo_dbh_object_t *pdo_obj;
 			if (new_zval_object) {
-- 
2.47.1.windows.1

[iliaal]

The driver check only rejects re-constructing a handle that already finished. The three cases the PR targets are reentry during construction, before the driver is attached, so dbh->driver is still NULL and a driver-only guard lets them through: a uri: DSN whose stream wrapper calls __construct() again on the still-constructing object, a failed persistent connect() whose __destruct() does the same while unwinding, and retrying a persistent construct that failed after the persistent handle was swapped in. That's why the flag is set at the start of construction and cleared on success, rather than keyed on driver.

I ran your patch against the PR's test: the two reconstruct cases throw as expected, but the three reentrant ones aren't rejected. The uri: one lets the inner __construct() win and then throws uncaught on the outer call; the retry and destructor cases just proceed.

pefree(dbh, dbh->is_persistent) is a correct fix for the free itself (after a failed persistent attempt the inner handle is the persistent one, so efree is the wrong allocator), but single-shot rejects those reconstructs and retries before that free is reached, so it isn't needed once reentry is blocked.