Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Sep 22, 2024

We need to release the fcall info cache instead of destroying it.

We need to release the fcall info cache instead of destroying it.
@Girgias
Copy link
Member

Girgias commented Sep 22, 2024

Do you have a test case showcasing this? As it feels like dtor is correct due to the F ZPP modifier.

@cmb69
Copy link
Member Author

cmb69 commented Sep 22, 2024

Do you have a test case showcasing this?

See #15986. I've noticed that when running https://github.com/php/php-src/blob/master/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback_trampoline_no_leak.phpt under ASan (therefore I didn't add a new test case).

@cmb69
Copy link
Member Author

cmb69 commented Sep 22, 2024

The problem with the dtor in this case is, that it decreases the refcount of the ($t) object.

@Girgias
Copy link
Member

Girgias commented Sep 22, 2024

The problem with the dtor in this case is, that it decreases the refcount of the ($t) object.

Ah, right, okay I see the problem. I was worried this meant all cases where the ZPP F is used has this problem, which wouldn't be great.

Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks !

@cmb69 cmb69 closed this in 2b90acb Sep 22, 2024
@cmb69 cmb69 deleted the cmb/gh15986 branch September 22, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Double-free due to Pdo\Pgsql::setNoticeCallback()

3 participants