Skip to content

Fix #22986: odbc_connect() may reuse persistent connection #6223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Sep 27, 2020

odbc_connect() should not reuse persistent connections, since that
prohibits multiple concurrent connections, which are occasionally
desireable. We fix that by no longer looking for already cached
connections when odbc_connect() is called, and instead creating a new
connection instead.


I'm slightly concerned about Shane's comment that there would be segfaults in case of multiple connections. I haven't found issues during quick testing, so maybe these have long been resolved. If we want to play it safe, we may postpone this to PHP 8.1, to have a longer QA cycle.

`odbc_connect()` should not reuse persistent connections, since that
prohibits multiple concurrent connections, which are occasionally
desireable.  We fix that by no longer looking for already cached
connections when `odbc_connect()` is called, and instead creating a new
connection instead.
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

This kind of non-persistent connection reuse is present in a couple of DB extensions. I remember fighting with it in the now removed interbase driver. The whole concept seems a leftover from times immemorial, where people were incapable of opening a connection once and passing it down to where it is needed... So yes, let's get rid of it, especially if it breaks other use-cases.

Can't comment on the crashing comment though.

@php-pulls php-pulls closed this in 9f5a771 Sep 29, 2020
@cmb69 cmb69 deleted the cmb/22986 branch September 29, 2020 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants