From b425175333f2d6f00213ca3c4e2dc89f726789ca Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 8 Nov 2026 11:17:57 -0500 Subject: [PATCH] ext/dba: Fix dba_close() not closing a dba_popen() connection dba_close() looked up the persistent entry with connection->info after dba_close_connection() had cleared it, so the entry was never removed and the database file and its lock stayed held. Count the connections on the entry's refcount again, as PHP 8.3 did, and close the handle when dba_close() releases the last one, unless another persistent handle to the same file shares its stream. Lock conflict checks now also see persistent handles still in use, since closing one of two connections with the same key removed the other's DBA_G(connections) entry. --- NEWS | 4 ++ ext/dba/dba.c | 45 +++++++++++++++++-- ext/dba/tests/dba_popen_close.phpt | 70 ++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 ext/dba/tests/dba_popen_close.phpt diff --git a/NEWS b/NEWS index facc4a1d0031..a00bf9fc025d 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,10 @@ PHP NEWS - Curl: . Fixed curl_reset() retaining CURLOPT_PRIVATE after reset. (lacatoire) +- DBA: + . Fixed dba_close() not closing a database opened with dba_popen(). + (Ilia Alshanetsky) + - DOM: . Fixed bug GH-25268 (NULL php_libxml_ref_obj dereference in dom_objects_store_clone_obj when cloning Dom\Notation). (David Carlier) diff --git a/ext/dba/dba.c b/ext/dba/dba.c index 29e996e4f320..b64fdc2ce400 100644 --- a/ext/dba/dba.c +++ b/ext/dba/dba.c @@ -271,6 +271,22 @@ static void dba_close_info(dba_info *info) info = NULL; } +static bool dba_info_shares_stream(const dba_info *info) +{ + ZEND_HASH_FOREACH_PTR(&EG(persistent_list), zend_resource *le) { + if (le->type != le_pdb || le->ptr == NULL || le->ptr == info) { + continue; + } + const dba_info *other = le->ptr; + if ((info->fp && (info->fp == other->fp || info->fp == other->lock.fp)) + || (info->lock.fp && (info->lock.fp == other->fp || info->lock.fp == other->lock.fp))) { + return true; + } + } ZEND_HASH_FOREACH_END(); + + return false; +} + static int remove_pconnection_from_list(zval *zv, void *p) { zend_resource *le = Z_RES_P(zv); @@ -297,6 +313,11 @@ static void dba_close_connection(dba_connection *connection) if (!persistent) { dba_close_info(connection->info); + } else if (connection->hash) { + zend_resource *le = zend_hash_find_ptr(&EG(persistent_list), connection->hash); + if (le) { + GC_DELREF(le); + } } connection->info = NULL; @@ -501,6 +522,15 @@ static dba_info *php_dba_find(const zend_string *path) } } ZEND_HASH_FOREACH_END(); + ZEND_HASH_FOREACH_PTR(&EG(persistent_list), zend_resource *le) { + if (le->type == le_pdb && GC_REFCOUNT(le) > 1) { + dba_info *info = le->ptr; + if (info && zend_string_equals(path, info->path)) { + return info; + } + } + } ZEND_HASH_FOREACH_END(); + return NULL; } /* }}} */ @@ -596,6 +626,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent) efree(resource_key); RETURN_FALSE; } + GC_ADDREF(le); object_init_ex(return_value, dba_connection_ce); dba_connection *connection = Z_DBA_CONNECTION_P(return_value); @@ -951,13 +982,15 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent) connection->info->hnd = hptr; if (persistent) { - if (zend_register_persistent_resource(resource_key, resource_key_len, connection->info, le_pdb) == NULL) { + zend_resource *le = zend_register_persistent_resource(resource_key, resource_key_len, connection->info, le_pdb); + if (le == NULL) { php_error_docref(NULL, E_WARNING, "Could not register persistent resource"); efree(resource_key); dba_close_connection(connection); zval_ptr_dtor(return_value); RETURN_FALSE; } + GC_ADDREF(le); } zend_hash_add_new(&DBA_G(connections), connection->hash, return_value); @@ -1000,12 +1033,16 @@ PHP_FUNCTION(dba_close) connection = Z_DBA_CONNECTION_P(id); CHECK_DBA_CONNECTION(connection->info); - bool persistent = connection->info->flags & DBA_PERSISTENT; + dba_info *info = connection->info; + zend_resource *le = NULL; + if ((info->flags & DBA_PERSISTENT) && connection->hash) { + le = zend_hash_find_ptr(&EG(persistent_list), connection->hash); + } dba_close_connection(connection); - if (persistent) { - zend_hash_apply_with_argument(&EG(persistent_list), remove_pconnection_from_list, (void *) connection->info); + if (le && GC_REFCOUNT(le) == 1 && !dba_info_shares_stream(info)) { + zend_hash_apply_with_argument(&EG(persistent_list), remove_pconnection_from_list, (void *) info); } } /* }}} */ diff --git a/ext/dba/tests/dba_popen_close.phpt b/ext/dba/tests/dba_popen_close.phpt new file mode 100644 index 000000000000..9e9b9cb78a19 --- /dev/null +++ b/ext/dba/tests/dba_popen_close.phpt @@ -0,0 +1,70 @@ +--TEST-- +dba_close() releases a persistent connection once no other connection uses it +--EXTENSIONS-- +dba +--SKIPIF-- + +--CONFLICTS-- +dba +--FILE-- + +--CLEAN-- + +--EXPECT-- +first still open: value +lock free after closing one of two: bool(false) +lock free after closing both: bool(true) +reused after implicit release: value +lock free after closing the reused connection: bool(true) +bool(false) +conflict seen after closing the earlier connection: dba_open(): Driver initialization failed for handler: flatfile: Unable to establish lock (database file already open) +connection sharing the stream still reads: value