Repository navigation
Conversation
Only call it for the last PDO object, like the rollback before it.
…and plans. Fix php#24461 Keep the rest of DISCARD ALL, and reset the connection if it fails.
devnexen
changed the title
…repared. Track prepared statement names on the handle and close the leftovers.
|
Thank you. I had reached the same conclusion after looking at the issue. Although I haven’t reviewed every single change, the approach to handling multiple persistent connections and the criteria for determining which state to discard both make sense to me. |
ArtUkrainskiy
left a comment
ArtUkrainskiy
left a comment
There was a problem hiding this comment.
There's one leak with libpq < 17. If a statement is closed inside an aborted transaction, DEALLOCATE fails and the statement stays on the server. The new reset doesn't clean it up anymore, DISCARD ALL did. Fix and repro inline, tests pass with ASan otherwise.
| PQclear(res); | ||
| } | ||
|
|
||
| if (H->prepared_stmts) { | ||
| zend_hash_str_del(H->prepared_stmts, S->stmt_name, strlen(S->stmt_name)); | ||
| } |
There was a problem hiding this comment.
If the close fails, the name is still removed here, so the reset never closes that statement. I'd only remove it when the close worked:
| PQclear(res); | |
| } | |
| if (H->prepared_stmts) { | |
| zend_hash_str_del(H->prepared_stmts, S->stmt_name, strlen(S->stmt_name)); | |
| } | |
| if (H->prepared_stmts && PQresultStatus(res) == PGRES_COMMAND_OK) { | |
| zend_hash_str_del(H->prepared_stmts, S->stmt_name, strlen(S->stmt_name)); | |
| } | |
| PQclear(res); | |
| } |
This prints 1 now and 0 with the change:
$opts = [PDO::ATTR_PERSISTENT => true, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_EMULATE_PREPARES => false];
$pdo = new PDO($dsn, null, null, $opts);
$pdo->beginTransaction();
$stmt = $pdo->prepare("SELECT 'leak'");
$stmt->execute();
try { $pdo->exec('SELECT 1/0'); } catch (PDOException $e) {}
$stmt = null;
$pdo->rollBack();
$pdo = null;
$pdo = new PDO($dsn, null, null, $opts);
echo $pdo->query("SELECT count(*) FROM pg_prepared_statements WHERE statement = 'SELECT ''leak'''")->fetchColumn();There was a problem hiding this comment.
applied your suggestions let me know if all work out for you
There was a problem hiding this comment.
Thanks, works for me!
Co-authored-by: Artem Ukrainskiy <art.ukrainskiy@gmail.com>
No description provided.