Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Nov 9, 2020

We must not assume that a statement handle is still valid, if the
environment handle already has been freed, which would happen if the
PDO connection object is destroyed before its associated statement
handles during shutdown.

We must not assume that a statement handle is still valid, if the
environment handle already has been freed, which would happen if the
PDO connection object is destroyed before its associated statement
handles during shutdown.
@cmb69 cmb69 added the Bug label Nov 9, 2020
@cmb69 cmb69 marked this pull request as draft November 10, 2020 15:56
@cmb69
Copy link
Member Author

cmb69 commented Nov 11, 2020

The problem (at least in the particular case reported by the OP) is that if zend.enable_gc=0 and fast_shutdown is triggered, the instance of Core is not destroyed, and then the DB handle is freed before the statement handle, which apparently can lead to a segfault. I cannot reproduce this on Windows, so this may actually be an implementation glitch of the driver or driver manager.

Anyhow, this patch avoids that by (a) guarding the statement dtor, and (b) avoiding to free the actual pdo_odbc_db_handle during shutdown. Obviously, this introduces a memory leak in the shutdown sequence (so it's not a real leak), but it will be reported in debug mode nonetheless.

I should mention that the same destruction sequence also happens with PDO_SQLite (and likely with other drivers as well), but nothing bad happens for me in this case either. So maybe we should just call this an upstream issue?

@cmb69 cmb69 marked this pull request as ready for review November 11, 2020 11:47
@cmb69
Copy link
Member Author

cmb69 commented Nov 17, 2020

The basic question is whether calling SQLFreeHandle() on a statement handle after the function has been called on the respective connection and environment handle is supposed to be a valid operation or not. Apparently, the Microsoft documentation doesn't answer that question.

@cmb69
Copy link
Member Author

cmb69 commented Nov 30, 2020

On further consideration, this looks like an upstream issue to me.

@cmb69 cmb69 closed this Nov 30, 2020
@cmb69 cmb69 deleted the cmb/79270 branch November 30, 2020 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant