Skip to content

PDO - Added PDO::disconnect and PDO::isConnected and refactored free object #19214

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: PHP-8.3
Choose a base branch
from
Open

PDO - Added PDO::disconnect and PDO::isConnected and refactored free object #19214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 120 additions & 53 deletions ext/pdo/pdo_dbh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -1027,8 +1040,6 @@ PHP_METHOD(PDO, errorCode)

ZEND_PARSE_PARAMETERS_NONE();

PDO_CONSTRUCT_CHECK;
Copy link
Member

Choose a reason for hiding this comment

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

question: why did you remove/not add the construct checks?

Copy link
Author

Choose a reason for hiding this comment

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

Here's the relevant section from my initial comment on this PR.

Both PDO::errorInfo() and PDO::errorCode() may now be called on an uninitialized PDO object, i.e. one without a defined driver member. This change is being done to permit use of both functions after PDO::disconnect() is called, and as a result of modifying the PDO_CONSTRUCT_CHECK macro to impose check on is_closed.

Both method implementations look to tolerate the absence of a defined driver member, with the exception of methods->fetch_err() invocation from within errorInfo(), where I've added the null check on methods member as safeguard.


if (dbh->query_stmt) {
RETURN_STRING(dbh->query_stmt->error_code);
}
Expand Down Expand Up @@ -1056,8 +1067,6 @@ PHP_METHOD(PDO, errorInfo)

ZEND_PARSE_PARAMETERS_NONE();

PDO_CONSTRUCT_CHECK;

array_init(return_value);

if (dbh->query_stmt) {
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand All @@ -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;
}
}
Expand Down
4 changes: 4 additions & 0 deletions ext/pdo/pdo_dbh.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand All @@ -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 {}

Expand Down
11 changes: 10 additions & 1 deletion ext/pdo/pdo_dbh_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions ext/pdo/php_pdo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(); \
Expand Down
2 changes: 2 additions & 0 deletions ext/pdo/php_pdo_driver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
Loading
Loading