Fix GH-21162: pg_connect() on error memory leak.#21165
Fix GH-21162: pg_connect() on error memory leak.#21165devnexen wants to merge 2 commits intophp:PHP-8.4from
Conversation
The PHP_PQ_ERROR macro calls php_error_docref() which triggers user error handlers thus libpq does not have the chance to clean the resources (and empty connections string are allowed) on failure thus we avoid this macro and delay the error handling after.
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Does it make sense to remove the PHP_PQ_ERROR macro?
|
No it is still used in other contexts. |
|
@devnexen Please check why this fails on Windows. https://github.com/php/php-src/actions/runs/22043379618/job/63687927421 Did you get e-mail notifications about failing pushed commits? If not, can you enable it here? https://github.com/settings/notifications 
|
Sorry I was very unavailable today I ll able to check in less than 1h. Cheers |
|
So it seems the error handler does not trigger on windows ? but since that s the whole point of the bug, should I just disable on windows, wdyt @iluuu1994 ? |
skipping on windows as the error handler does not seem to trigger.
|
I don't know, I don't even know what warning is being triggered, since the message is discarded. Do you know why this isn't triggered on Windows specifically? |
|
not entirely sure, what can help it to see if pg_connect with an empty string behaves differently on windows. on unix => attempts to open an unix socket => does not work => error. on windows I m not sure yet what is the workflow. |
making it fails early instead. close GH-21234
* PHP-8.4: ext/pgsql: fix GH-21165 unit test.
* PHP-8.5: ext/pgsql: fix GH-21165 unit test.
3 participants
The PHP_PQ_ERROR macro calls php_error_docref() which triggers user error handlers thus libpq does not have the chance to clean the resources (and empty connections string are allowed) on failure thus we avoid this macro and delay the error handling after.