Bug #75190 [Nab]: Impossible to close PDO connection when extending PdoStatement using $this

From: Date: Thu, 05 Oct 2017 23:20:54 +0000
Subject: Bug #75190 [Nab]: Impossible to close PDO connection when extending PdoStatement using $this
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-211541@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=75190&edit=1

 ID:                 75190
 User updated by:    ian at oshaughnessy dot cc
 Reported by:        ian at oshaughnessy dot cc
 Summary:            Impossible to close PDO connection when extending
                     PdoStatement using $this
 Status:             Not a bug
 Type:               Bug
 Package:            PDO related
 Operating System:   Debian 9
 PHP Version:        7.0.23
 Block user comment: N
 Private report:     N

 New Comment:

I guess my only issue with this being flagged as 'not a bug' is that you can cause a
memory leak by writing valid PHP code, which seems pretty buggy.

Side note, extending the Pdo library is the only option in our case, since composition means we
cannot use any 3rd party libs, since no external code base is going to want access to an
"IansPdoClass" wrapper object, they want the Pdo object.


Previous Comments:
------------------------------------------------------------------------
[2017-10-05 21:34:06] nikic@php.net

I remember fixing this issue... and indeed, the pdo_dbh get_gc handler returns def_stmt_ctor_args:
https://github.com/php/php-src/blob/master/ext/pdo/pdo_dbh.c#L1382

As such this should not be an actual leak (like it was on PHP 5), but the destruction is delayed
until the next cycle GC. Explicitly calling gc_collect_cycles() may resolve the problem.

------------------------------------------------------------------------
[2017-10-05 20:42:00] adambaratz@php.net

You could pass an empty array for the ctor args. It's not strictly required that you pass a
reference to the PDO object. I'm not sure this is a PDO issue, so much as a side effect of
PHP's reference counting.

For the sake of clarity, I'd recommend avoiding doing what you're doing here. Basically,
because it's easy to create situations like this one. If you want the statement to have a
reference to the connection, I'd recommend composition over inheritance. That is, creating your
own PDO and PDOStatement classes that wrap the built-ins.

There is the separate point that it's awkward that you can't close a connection without
destructing the PDO object. If that's an issue you want to address, please file a feature
request ticket. Or better still, put together an RFC to hash out the details with the internals
community.

------------------------------------------------------------------------
[2017-09-11 15:58:13] ian at oshaughnessy dot cc

Description:
------------
The only way to close a PDO connection is to set the resource variable to null (thereby destroying
the PDO object); however, when extending the PdoStatement class, it's required that $this is
passed as an option to the PDO object for Pdo::ATTR_STATEMENT_CLASS.

In doing this, a reference to the PDO object is created, making it impossible to destroy the PDO
object since a lingering reference continues to exist after $pdo is set to null.

The only way to fix this issue is to use Pdo::setAttribute() after the PDO parent constructor has
been run.

Test script:
---------------
<?php

echo "Test 1 (PdoTestBroken)\n";
echo "new PdoTestBroken\n";
$pdo1 = new PdoTestBroken($dsn, $username, $password, []);
echo "Destroy \$pdo\n";
$pdo1 = null;
echo "----------------------------\n\n";

echo "Test 2 (PdoTestWorks)\n";
echo "new PdoTestWorks\n";
$pdo2 = new PdoTestWorks($dsn, $username, $password, []);
echo "Destroy \$pdo\n";
$pdo2 = null;
echo "----------------------------\n\n";

class PdoTestBroken extends Pdo {

	public function __construct($dsn, $username, $passwd, $options) {
		echo "PdoTest1::__construct()\n";
		$options[self::ATTR_STATEMENT_CLASS] = [ 'PdoStatementTest', [ $this, ] ];
		parent::__construct($dsn, $username, $passwd, $options);
	}

	public function __destruct() {
		echo "PdoTest1::__destruct()\n";
	}
}

class PdoTestWorks extends Pdo {

	public function __construct($dsn, $username, $passwd, $options) {
		echo "PdoTest2::__construct()\n";
		$self = parent::__construct($dsn, $username, $passwd, $options);
		$this->setAttribute(PDO::ATTR_STATEMENT_CLASS, [ 'PdoStatementTest', [ $self, ] ]);
	}

	public function __destruct() {
		echo "PdoTest2::__destruct()\n";
	}
}

class PdoStatementTest extends PdoStatement {

}

Expected result:
----------------
The connection to close.


Actual result:
--------------
The connection does not close due to hidden reference variable.

Script Results:

Test 1 (PdoTestBroken)
new PdoTestBroken
PdoTest1::__construct()
Destroy $pdo
----------------------------

Test 2 (PdoTestWorks)
new PdoTestWorks
PdoTest2::__construct()
Destroy $pdo
PdoTest2::__destruct()
----------------------------

PdoTest1::__destruct()


------------------------------------------------------------------------



--
Edit this bug report at https://bugs.php.net/bug.php?id=75190&edit=1


Thread (6 messages)

« previous php.bugs (#211541) next »