Fix bug and add test for dba_open same file twice

[chschneider]

When doing dba_open twice with the same file then dba_open tries to insert the same key into its hash table which triggers an assertion in a debug build and causes memory corruption in production.

A small reproduction script is

<?php
$a = dba_open("ext/dba/tests/test.cdb", "r");
$b = dba_open("ext/dba/tests/test.cdb", "r");
var_dump($a, $b);

The solution I chose is to add a counter to the keys generated with dba_open (but no dba_popen) to avoid key collisions.

[Girgias]

Thank you! Just some questions and remarks.

Is this only a PHP 8.4 issue? Or does this also exist with 8.3? And if so, what is the behaviour prior to 8.4?

Because the solution might be to change the usage of zend_hash_add_new to the new or update variant.

[Girgias]

Suggested change

	$db_filename = __DIR__.'/test.cdb';
	$db_filename = __DIR__.'/dba_duplicateopen.cdb';

[chschneider]

What is the advantage of having a separate db test file? I double anyone is deleting test.cdb and I thought the name of the db is generic enough for reuse.

[chschneider]

Is this only a PHP 8.4 issue? Or does this also exist with 8.3? And if so, what is the behaviour prior to 8.4?

I didn’t manage to trigger it with 8.3.

The problem seems to have appeared with the rewrite from resources to objects for dba which happened with 8.4

Because the solution might be to change the usage of zend_hash_add_new to the new or update variant.

I’m not that deep into the topic but I was worried that it might cause problems with dba_close on the first connection but still using the second.

My first attempt was skipping the update but that didn’t work. My test case contains the open1/open2/close2/read1 for this reason.
But I can give update try even though
I am not sure how the first connection would be preserved/cleaned up.

[Girgias]

Is this only a PHP 8.4 issue? Or does this also exist with 8.3? And if so, what is the behaviour prior to 8.4?

I didn’t manage to trigger it with 8.3.

The problem seems to have appeared with the rewrite from resources to objects for dba which happened with 8.4

Because the solution might be to change the usage of zend_hash_add_new to the new or update variant.

I’m not that deep into the topic but I was worried that it might cause problems with dba_close on the first connection but still using the second.

My first attempt was skipping the update but that didn’t work. My test case contains the open1/open2/close2/read1 for this reason. But I can give update try even though I am not sure how the first connection would be preserved/cleaned up.

Right, probably the better solution then is to first check if the key exists in the connection hashtable, and increase the refcount of the object before returning the already opened connection?

Or is our use case specifically about having multiple independent connections open?

[chschneider]

Right, probably the better solution then is to first check if the key exists in the connection hashtable, and increase the refcount of the object before returning the already opened connection?

Or is our use case specifically about having multiple independent connections open?

I didn't really look into what dba_close is doing exactly, so I wasn't sure if it is just about the refcount of the object or if dba_close would also have to be adapted. Opening a new connection seemed the easier and safer approach.

And I noted another issue: Not all parameters are used to generate the hash key (only persistence, path, mode and handler) and I felt a bit uncomfortable to reuse a connection which might have had different parameters (which is probably harmless and happening with dba_popen) but nevertheless...

I don't think the whole reusing of connections for dba_open (including why we actually have dba_popen) is really still needed in 2025. YMMV.