Skip to content

Fix #76804: OCI8 - correctly releasing persistent connections#3491

Closed
KoenigsKind wants to merge 1 commit intophp:PHP-7.1from
KoenigsKind:oci8-release-connection
Closed

Fix #76804: OCI8 - correctly releasing persistent connections#3491
KoenigsKind wants to merge 1 commit intophp:PHP-7.1from
KoenigsKind:oci8-release-connection

Conversation

@KoenigsKind
Copy link
Contributor

@KoenigsKind KoenigsKind commented Sep 4, 2018

When not using spool, creating a persistent connection, releasing the handle and recreating a persistent connection, leads to the second connection being an invalid resource.

How to reproduce

<?php
ini_set( 'display_errors', 'on' );
ini_set( 'log_errors', 'off' );
$use_wallet         = true;
$userName           = '/';
$password           = '';
$connection_string  = 'DB_CONNECTION_STRING';
$character_set      = 'AL32UTF8';
$session_mode       = OCI_CRED_EXT;

$conn = oci_pconnect( $userName, $password, $connection_string, $character_set, $session_mode );
var_dump( $conn );
$conn = null;
$conn = oci_pconnect( $userName, $password, $connection_string, $character_set, $session_mode );
var_dump( $conn );

A possible result would be:

resource(1) of type (oci8 connection)
resource(-1555988216) of type (Unknown)

Cause
This probably happens because the function php_oci_connection_release(...) currently doesn't set the connection->id to null (only when using spool). While the function php_oci_do_connect_ex(...) checks if (connection->id) { ... }, this check works for the first time, but not for the second time oci_pconnect(...) is being called.

Fix
Always performing a connection->id = NULL; fixed the problem for us.

@KalleZ
Copy link
Member

KalleZ commented Sep 4, 2018

Tagging Mr. OCI himself: @cjbj

@cmb69
Copy link
Member

cmb69 commented Sep 4, 2018

To quote the README:

Bug fixes do not require an RFC, but require a bugtracker ticket.

@KoenigsKind
Copy link
Contributor Author

KoenigsKind commented Sep 4, 2018

@cmb69 My bad, I only checked CONTRIBUTING.

It seems like there are already possible bug reports regarding this pull request:
#76804 describes the same problem with resource type being "unknown", can I use this bug report?
#74420 might be caused from the same problem.

@cmb69
Copy link
Member

cmb69 commented Sep 4, 2018

My bad, I only checked CONTRIBUTING.

No problem.

#76804 describes the same problem with resource type being "unknown", can I use this bug report?

LGTM

@KoenigsKind KoenigsKind changed the title oci8-bugfix: releasing persistent connections Fix #76804: OCI8 - correctly releasing persistent connections Sep 4, 2018
@KoenigsKind
Copy link
Contributor Author

KoenigsKind commented Sep 4, 2018

I tried twice adding the pull request to the bug report, as it's stated in CONTRIBUTING, but by clicking on save I just get redirected to a blank page...

@cmb69
Copy link
Member

cmb69 commented Sep 4, 2018

Sorry, forgot to mention https://bugs.php.net/bug.php?id=76079. Just add a comment with a link to this PR for now.

@krakjoe
Copy link
Member

krakjoe commented Sep 5, 2018

We'll await noise from @cjbj ...

@cjbj
Copy link
Contributor

cjbj commented Sep 5, 2018

I'm mostly back from vacation. I'll take a look.

@cmb69
Copy link
Member

cmb69 commented Nov 5, 2018

@cjbj ping

@php-pulls
Copy link

php-pulls commented Dec 8, 2018

Comment on behalf of sixd at php.net:

Merged - thanks!

@php-pulls php-pulls closed this Dec 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug Waiting on Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@KoenigsKind @KalleZ @cmb69 @krakjoe @cjbj @php-pulls