diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 4ac271fefd4bf..b51777274ca35 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -149,7 +149,7 @@ PDO_API void pdo_handle_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt) /* {{{ */ } ZVAL_UNDEF(&info); - if (dbh->methods->fetch_err) { + if (dbh->methods && dbh->methods->fetch_err) { zval *item; array_init(&info); @@ -221,6 +221,21 @@ static char *dsn_from_uri(char *uri, char *buf, size_t buflen) /* {{{ */ } /* }}} */ +/* Fetch the registered persistent PDO object for the given key */ +static pdo_dbh_t *pdo_list_entry_from_key(const char *hashkey, size_t len) +{ + pdo_dbh_t *pdbh = NULL; + zend_resource *le; + + if ((le = zend_hash_str_find_ptr(&EG(persistent_list), hashkey, len)) != NULL) { + if (le->type == php_pdo_list_entry()) { + pdbh = (pdo_dbh_t*)le->ptr; + } + } + + return pdbh; +} + /* {{{ */ PHP_METHOD(PDO, __construct) { @@ -297,7 +312,6 @@ PHP_METHOD(PDO, __construct) if (options) { int plen = 0; char *hashkey = NULL; - zend_resource *le; pdo_dbh_t *pdbh = NULL; zval *v; @@ -320,36 +334,22 @@ PHP_METHOD(PDO, __construct) if (is_persistent) { /* let's see if we have one cached.... */ - if ((le = zend_hash_str_find_ptr(&EG(persistent_list), hashkey, plen)) != NULL) { - if (le->type == php_pdo_list_entry()) { - pdbh = (pdo_dbh_t*)le->ptr; - - /* is the connection still alive ? */ - if (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) { - /* nope... need to kill it */ - pdbh->refcount--; - zend_list_close(le); - pdbh = NULL; - } - } - } - - if (pdbh) { - call_factory = 0; - } else { + pdbh = pdo_list_entry_from_key(hashkey, plen); + /* is the connection still alive ? */ + if (!pdbh || pdbh->is_closed || + (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh))) { /* need a brand new pdbh */ pdbh = pecalloc(1, sizeof(*pdbh), 1); - pdbh->refcount = 1; pdbh->is_persistent = 1; pdbh->persistent_id = pemalloc(plen + 1, 1); memcpy((char *)pdbh->persistent_id, hashkey, plen+1); pdbh->persistent_id_len = plen; pdbh->def_stmt_ce = dbh->def_stmt_ce; + } else { + /* found viable dbh persisted */ + call_factory = 0; } - } - - if (pdbh) { efree(dbh); /* switch over to the persistent one */ Z_PDO_OBJECT_P(object)->inner = pdbh; @@ -393,6 +393,8 @@ PHP_METHOD(PDO, __construct) /* register in the persistent list etc. */ /* we should also need to replace the object store entry, since it was created with emalloc */ + /* if a resource is already registered, then it failed a liveness check + * and will be replaced, prompting destruct. */ if ((zend_register_persistent_resource( (char*)dbh->persistent_id, dbh->persistent_id_len, dbh, php_pdo_list_entry())) == NULL) { php_error_docref(NULL, E_ERROR, "Failed to register persistent entry"); @@ -422,9 +424,6 @@ PHP_METHOD(PDO, __construct) } /* the connection failed; things will tidy up in free_storage */ - if (is_persistent) { - dbh->refcount--; - } /* XXX raise exception */ zend_restore_error_handling(&zeh); @@ -587,6 +586,9 @@ PHP_METHOD(PDO, prepare) static bool pdo_is_in_transaction(pdo_dbh_t *dbh) { + if (dbh->is_closed) { + return false; + } if (dbh->methods->in_transaction) { return dbh->methods->in_transaction(dbh); } @@ -684,6 +686,17 @@ PHP_METHOD(PDO, inTransaction) } /* }}} */ +/* {{{ Determine if connected */ +PHP_METHOD(PDO, isConnected) +{ + pdo_dbh_t *dbh = Z_PDO_DBH_P(ZEND_THIS); + + ZEND_PARSE_PARAMETERS_NONE(); + + RETURN_BOOL(!dbh->is_closed); +} +/* }}} */ + PDO_API bool pdo_get_long_param(zend_long *lval, zval *value) { switch (Z_TYPE_P(value)) { @@ -1027,8 +1040,6 @@ PHP_METHOD(PDO, errorCode) ZEND_PARSE_PARAMETERS_NONE(); - PDO_CONSTRUCT_CHECK; - if (dbh->query_stmt) { RETURN_STRING(dbh->query_stmt->error_code); } @@ -1056,8 +1067,6 @@ PHP_METHOD(PDO, errorInfo) ZEND_PARSE_PARAMETERS_NONE(); - PDO_CONSTRUCT_CHECK; - array_init(return_value); if (dbh->query_stmt) { @@ -1068,7 +1077,8 @@ PHP_METHOD(PDO, errorInfo) if(!strncmp(dbh->error_code, PDO_ERR_NONE, sizeof(PDO_ERR_NONE))) goto fill_array; } - if (dbh->methods->fetch_err) { + /* Driver-implemented error is not available once database is shutdown. */ + if (dbh->methods && dbh->methods->fetch_err) { dbh->methods->fetch_err(dbh, dbh->query_stmt, return_value); } @@ -1366,23 +1376,53 @@ void pdo_dbh_init(int module_number) pdo_dbh_object_handlers.get_gc = dbh_get_gc; } -static void dbh_free(pdo_dbh_t *dbh, bool free_persistent) +/* Disconnect from the database and free associated driver. */ +static void dbh_shutdown(pdo_dbh_t *dbh) { - int i; + if (dbh->driver_data && dbh->methods && dbh->methods->rollback && pdo_is_in_transaction(dbh)) { + dbh->methods->rollback(dbh); + dbh->in_txn = false; + } - if (dbh->query_stmt) { - zval_ptr_dtor(&dbh->query_stmt_zval); - dbh->query_stmt = NULL; + if (dbh->methods) { + dbh->methods->closer(dbh); } - if (dbh->is_persistent) { + /* Do not permit reference to driver methods to remain past closer(), which + * is responsible for both disconnecting the db and free-ing allocations. + * Ideally, this would only disconnect the database, not free the handle. */ + dbh->methods = NULL; + dbh->is_closed = true; +} + +/* {{{ Disconnect from the database. */ +PHP_METHOD(PDO, disconnect) +{ + pdo_dbh_t *dbh = Z_PDO_DBH_P(ZEND_THIS); + + ZEND_PARSE_PARAMETERS_NONE(); + + PDO_DBH_CLEAR_ERR(); + PDO_CONSTRUCT_CHECK; + + dbh_shutdown(dbh); + + PDO_HANDLE_DBH_ERR(); + + RETURN_TRUE; +} +/* }}} */ + +/* Free the database when the last pdo object referencing it is freed + * or when it has been registered as a php resource, i.e. is persistent, + * and the resource is destructed, whichever comes last. */ +static void dbh_free(pdo_dbh_t *dbh) +{ + int i; + #if ZEND_DEBUG - ZEND_ASSERT(!free_persistent || (dbh->refcount == 1)); + ZEND_ASSERT(dbh->refcount == 0); #endif - if (!free_persistent && (--dbh->refcount)) { - return; - } - } if (dbh->methods) { dbh->methods->closer(dbh); @@ -1416,25 +1456,48 @@ static void dbh_free(pdo_dbh_t *dbh, bool free_persistent) pefree(dbh, dbh->is_persistent); } +/* Whether the given database handler is presently registered as a resource. */ +static bool pdo_is_persisted(pdo_dbh_t *dbh) +{ + pdo_dbh_t *pdbh = NULL; + + if (dbh->persistent_id != NULL) { + pdbh = pdo_list_entry_from_key(dbh->persistent_id, dbh->persistent_id_len); + return dbh == pdbh; + } + + return false; +} + static void pdo_dbh_free_storage(zend_object *std) { pdo_dbh_t *dbh = php_pdo_dbh_fetch_inner(std); /* dbh might be null if we OOMed during object initialization. */ - if (!dbh) { - return; - } + if (dbh) { + if (dbh->is_persistent && dbh->methods && dbh->methods->persistent_shutdown) { + dbh->methods->persistent_shutdown(dbh); + } - if (dbh->driver_data && dbh->methods && dbh->methods->rollback && pdo_is_in_transaction(dbh)) { - dbh->methods->rollback(dbh); - dbh->in_txn = false; - } + /* stmt is not persistent, even if dbh is, so it must be freed with pdo. + * Consider copying stmt error code to dbh at this point, seemingly the reason + * that the stmt is even being held, or even better, to do that at the time of + * error and remove the reference altogether. */ + if (dbh->query_stmt) { + zval_ptr_dtor(&dbh->query_stmt_zval); + dbh->query_stmt = NULL; + } - if (dbh->is_persistent && dbh->methods && dbh->methods->persistent_shutdown) { - dbh->methods->persistent_shutdown(dbh); + /* a persisted dbh will be freed when the resource is destructed. */ + if (!(--dbh->refcount) && !pdo_is_persisted(dbh)) { + if (!dbh->is_closed) { + dbh_shutdown(dbh); + } + dbh_free(dbh); + } } + zend_object_std_dtor(std); - dbh_free(dbh, 0); } zend_object *pdo_dbh_new(zend_class_entry *ce) @@ -1447,6 +1510,7 @@ zend_object *pdo_dbh_new(zend_class_entry *ce) rebuild_object_properties(&dbh->std); dbh->inner = ecalloc(1, sizeof(pdo_dbh_t)); dbh->inner->def_stmt_ce = pdo_dbstmt_ce; + dbh->inner->refcount++; return &dbh->std; } @@ -1457,7 +1521,10 @@ ZEND_RSRC_DTOR_FUNC(php_pdo_pdbh_dtor) /* {{{ */ { if (res->ptr) { pdo_dbh_t *dbh = (pdo_dbh_t*)res->ptr; - dbh_free(dbh, 1); + if (!dbh->refcount) { + /* do not free if still referenced by pdo */ + dbh_free(dbh); + } res->ptr = NULL; } } diff --git a/ext/pdo/pdo_dbh.stub.php b/ext/pdo/pdo_dbh.stub.php index 45364147230f4..6dca99e5dea71 100644 --- a/ext/pdo/pdo_dbh.stub.php +++ b/ext/pdo/pdo_dbh.stub.php @@ -394,6 +394,8 @@ public function beginTransaction(): bool {} /** @tentative-return-type */ public function commit(): bool {} + public function disconnect(): bool {} + /** @tentative-return-type */ public function errorCode(): ?string {} @@ -412,6 +414,8 @@ public static function getAvailableDrivers(): array {} /** @tentative-return-type */ public function inTransaction(): bool {} + public function isConnected(): bool {} + /** @tentative-return-type */ public function lastInsertId(?string $name = null): string|false {} diff --git a/ext/pdo/pdo_dbh_arginfo.h b/ext/pdo/pdo_dbh_arginfo.h index afbbc935548e5..2de317af0eed9 100644 --- a/ext/pdo/pdo_dbh_arginfo.h +++ b/ext/pdo/pdo_dbh_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 7dcba884671fd90b891fab7e3f0d4cc9a4ac76a1 */ + * Stub hash: ba3a517f23ba76d450e933b2fdd33b30e73f8707 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO___construct, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, dsn, IS_STRING, 0) @@ -13,6 +13,9 @@ ZEND_END_ARG_INFO() #define arginfo_class_PDO_commit arginfo_class_PDO_beginTransaction +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_PDO_disconnect, 0, 0, _IS_BOOL, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_errorCode, 0, 0, IS_STRING, 1) ZEND_END_ARG_INFO() @@ -31,6 +34,8 @@ ZEND_END_ARG_INFO() #define arginfo_class_PDO_inTransaction arginfo_class_PDO_beginTransaction +#define arginfo_class_PDO_isConnected arginfo_class_PDO_disconnect + ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_MASK_EX(arginfo_class_PDO_lastInsertId, 0, 0, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, name, IS_STRING, 1, "null") ZEND_END_ARG_INFO() @@ -62,12 +67,14 @@ ZEND_END_ARG_INFO() ZEND_METHOD(PDO, __construct); ZEND_METHOD(PDO, beginTransaction); ZEND_METHOD(PDO, commit); +ZEND_METHOD(PDO, disconnect); ZEND_METHOD(PDO, errorCode); ZEND_METHOD(PDO, errorInfo); ZEND_METHOD(PDO, exec); ZEND_METHOD(PDO, getAttribute); ZEND_METHOD(PDO, getAvailableDrivers); ZEND_METHOD(PDO, inTransaction); +ZEND_METHOD(PDO, isConnected); ZEND_METHOD(PDO, lastInsertId); ZEND_METHOD(PDO, prepare); ZEND_METHOD(PDO, query); @@ -80,12 +87,14 @@ static const zend_function_entry class_PDO_methods[] = { ZEND_ME(PDO, __construct, arginfo_class_PDO___construct, ZEND_ACC_PUBLIC) ZEND_ME(PDO, beginTransaction, arginfo_class_PDO_beginTransaction, ZEND_ACC_PUBLIC) ZEND_ME(PDO, commit, arginfo_class_PDO_commit, ZEND_ACC_PUBLIC) + ZEND_ME(PDO, disconnect, arginfo_class_PDO_disconnect, ZEND_ACC_PUBLIC) ZEND_ME(PDO, errorCode, arginfo_class_PDO_errorCode, ZEND_ACC_PUBLIC) ZEND_ME(PDO, errorInfo, arginfo_class_PDO_errorInfo, ZEND_ACC_PUBLIC) ZEND_ME(PDO, exec, arginfo_class_PDO_exec, ZEND_ACC_PUBLIC) ZEND_ME(PDO, getAttribute, arginfo_class_PDO_getAttribute, ZEND_ACC_PUBLIC) ZEND_ME(PDO, getAvailableDrivers, arginfo_class_PDO_getAvailableDrivers, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) ZEND_ME(PDO, inTransaction, arginfo_class_PDO_inTransaction, ZEND_ACC_PUBLIC) + ZEND_ME(PDO, isConnected, arginfo_class_PDO_isConnected, ZEND_ACC_PUBLIC) ZEND_ME(PDO, lastInsertId, arginfo_class_PDO_lastInsertId, ZEND_ACC_PUBLIC) ZEND_ME(PDO, prepare, arginfo_class_PDO_prepare, ZEND_ACC_PUBLIC) ZEND_ME(PDO, query, arginfo_class_PDO_query, ZEND_ACC_PUBLIC) diff --git a/ext/pdo/php_pdo.h b/ext/pdo/php_pdo.h index 023372ebc8e6e..6567ad785a608 100644 --- a/ext/pdo/php_pdo.h +++ b/ext/pdo/php_pdo.h @@ -56,6 +56,10 @@ PHP_MINFO_FUNCTION(pdo); #define LONG_CONST(c) (zend_long) c #define PDO_CONSTRUCT_CHECK \ + if (dbh->is_closed) { \ + zend_throw_exception_ex(php_pdo_get_exception(), 0, "connection is closed"); \ + RETURN_THROWS(); \ + } \ if (!dbh->driver) { \ zend_throw_error(NULL, "PDO object is not initialized, constructor was not called"); \ RETURN_THROWS(); \ diff --git a/ext/pdo/php_pdo_driver.h b/ext/pdo/php_pdo_driver.h index c832284627750..8fe357343d420 100644 --- a/ext/pdo/php_pdo_driver.h +++ b/ext/pdo/php_pdo_driver.h @@ -478,6 +478,8 @@ struct _pdo_dbh_t { /* persistent hash key associated with this handle */ const char *persistent_id; size_t persistent_id_len; + + /* counter of _pdo_dbh_object_t referencing this handle */ unsigned int refcount; /* driver specific "class" methods for the dbh and stmt */ diff --git a/ext/pdo/tests/bug_63343.phpt b/ext/pdo/tests/bug_63343.phpt new file mode 100644 index 0000000000000..9775c4d000715 --- /dev/null +++ b/ext/pdo/tests/bug_63343.phpt @@ -0,0 +1,47 @@ +--TEST-- +PDO Common: Bug #63343 (Commit failure for repeated persistent connection) +--EXTENSIONS-- +pdo +--SKIPIF-- +beginTransaction(); +} catch (PDOException $exception) { + die("skip not relevant for driver - does not permit transactions"); +} +?> +--FILE-- + true))); + +$db1 = PDOTest::factory('PDO', false); +$db1->beginTransaction(); +var_dump($db1->inTransaction()); +/* db2 should assume persistent conn, including txn state */ +$db2 = PDOTest::factory('PDO', false); +var_dump($db2->inTransaction()); +/* destructing db1 should not rollback db2 */ +$db1 = null; +var_dump($db2->inTransaction()); +$db2 = null; +/* db3 should assume persistent conn, despite no remaining PDOs */ +$db3 = PDOTest::factory('PDO', false); +var_dump($db3->inTransaction()); +var_dump($db3->commit()); + +?> +--EXPECTF-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) diff --git a/ext/pdo/tests/pdo_040.phpt b/ext/pdo/tests/pdo_040.phpt new file mode 100644 index 0000000000000..3418d4fb42d48 --- /dev/null +++ b/ext/pdo/tests/pdo_040.phpt @@ -0,0 +1,89 @@ +--TEST-- +PDO Common: Explicit disconnect of common and persistent PDO +--EXTENSIONS-- +pdo +--SKIPIF-- +beginTransaction(); +} catch (PDOException $exception) { + die("skip not relevant for driver - does not permit transactions"); +} +?> +--FILE-- +isConnected()); +$db2 = PDOTest::factory('PDO', false); +var_dump($db2->isConnected()); +putenv("PDOTEST_ATTR=" . serialize(array(PDO::ATTR_PERSISTENT => true))); +$pdb3 = PDOTest::factory('PDO', false); +var_dump($pdb3->isConnected()); +$pdb4 = PDOTest::factory('PDO', false); +var_dump($pdb4->isConnected()); + +/* disconnect regular PDO, confirm other PDOs still connected */ +var_dump($db1->disconnect()); +var_dump($db1->isConnected()); +try { + $db1->beginTransaction(); +} catch (PDOException $e) { + echo $e->getMessage(), \PHP_EOL; +} +var_dump($db2->isConnected()); +var_dump($pdb3->isConnected()); + +/* disconnect persistent PDO, confirm other persistent PDO disconnected */ +var_dump($pdb3->disconnect()); +var_dump($pdb3->isConnected()); +var_dump($pdb4->isConnected()); +try { + $pdb4->disconnect(); +} catch (PDOException $e) { + echo $e->getMessage(), \PHP_EOL; +} + +/* new persistent PDO should prompt new connection */ +$pdb5 = PDOTest::factory('PDO', false); +var_dump($pdb5->isConnected()); +var_dump($pdb5->inTransaction()); +var_dump($pdb5->beginTransaction()); + +$db1 = null; +$db2 = null; /* trigger shutdown without explicit disconnect */ +$pdb3 = null; +$pdb4 = null; +$pdb5 = null; /* destruct should not disconnect */ + +/* no PDOs remain, but persistent connection should remain */ +$pdb6 = PDOTest::factory('PDO', false); +var_dump($pdb6->inTransaction()); + +?> +--EXPECTF-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(false) +connection is closed +bool(true) +bool(true) +bool(true) +bool(false) +bool(false) +connection is closed +bool(true) +bool(false) +bool(true) +bool(true) diff --git a/ext/pdo_mysql/tests/pdo_mysql_interface.phpt b/ext/pdo_mysql/tests/pdo_mysql_interface.phpt index aeb441bc0363a..679337c90b401 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_interface.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_interface.phpt @@ -30,6 +30,8 @@ if (false == MySQLPDOTest::detect_transactional_mysql_engine($db)) 'getAttribute' => true, 'quote' => true, 'inTransaction' => true, + 'disconnect' => true, + 'isConnected' => true, 'getAvailableDrivers' => true, ); $classname = get_class($db);