Fix foreach after the garbage collector is invoked

[danog]

Fixes #1776, https://jira.mongodb.org/browse/PHPC-2505, and the equivalent bug triggered when assigning dynamic properties and then unsetting them again.

The underlying issue in both cases is the invocation of zend_std_get_properties, which populates the properties hashtable in the zend object (not in the interned mongo one).

This, in turn, causes the iteration logic to completely skip iteration due to the if (zend_hash_num_elements(properties) == 0) { check in the ZEND_FE_RESET_R_SPEC_CV_HANDLER

The fix adds custom handlers for get_property, write_property, unset_property, has_property, get_property_ptr_ptr which use a new interned php_properties hashtable, instead of the zend hashtable property.

The zend_gc handler is also modified to invoke get_properties instead of zend_std_get_properties.

The overall implementation mostly works as expected, the only issue is some strange refcounting behavior within the garbage collector on the hashtable returned by get_gc, which sometimes increments the refcount without decrementing it again, leading to a leak of the interned properties hashtable (detectable using ASAN); I have asked clarifications regarding that, marking the PR as draft in the meantime.

There's also a leftover issue with foreach loops not exiting for whatever reason, still looking into that.

[danog]

Fixed up everything!

[danog]

The codebase was successfully formatted using scripts/clang-format.sh locally with clang-format 20, not sure why is it complaining here; can't access the evergreen tests so I can't see what's wrong there

[alcaeus]

I can take a look at the clang-format issue.

It looks like there is a memory leak in some tests:

[2025/07/28 09:44:20.753] FAILURE:  (FAILED)
[2025/07/28 09:44:20.753] --
[2025/07/28 09:44:20.753]      same serverConnectionId as last commandStarted: yes
[2025/07/28 09:44:20.753]      int(%d)
[2025/07/28 09:44:20.753]      OK: Got MongoDB\Driver\Exception\CommandException
[2025/07/28 09:44:20.753] 007+ [Mon Jul 28 07:43:20 2025]  Script:  '/data/mci/c0332320a42282d79356368a6fe838a7/src/tests/apm/commandFailedEvent-getServerConnectionId-001.php'
[2025/07/28 09:44:20.753] 008+ /data/mci/5683cf19da3859207a5bbd4900e33449/src/src/MongoDB/Manager.c(253) :  Freeing 0x00007f3fcc07c180 (56 bytes), script=/data/mci/c0332320a42282d79356368a6fe838a7/src/tests/apm/commandFailedEvent-getServerConnectionId-001.php
[2025/07/28 09:44:20.753] 009+ === Total 1 memory leaks detected ===

Our tests on GHA don't use debug versions of PHP, so these leaks are not reported there. The offending line in Manager.c allocates the intern->subscribers hash table:

mongo-php-driver/src/MongoDB/Manager.c

Line 253 in ae08bbc

ALLOC_HASHTABLE(intern->subscribers);

I can see that in a number of classes, you have replaced the calls to zend_hash_destroy followed by FREE_HASHTABLE with a single call to zend_hash_release, but left just a single call to zend_hash_destroy in Manager.c.

If you are testing with a debug version of PHP locally, I recommend calling ./configure with the --enable-mongodb-developer-flags switch to enable memory leak detection and other development checks.

[danog]

I simply compiled both php and the extension with ASAN/LSAN, which is even better than PHP's builtin leak detection, but the issue is that there are many unfixed leaks in PHP itself when dealing with fatal errors (such as the invalid class extension tests, etc), so I just focused on finding leaks in the GC/cycle tests, that's why I missed this one :)

Re: the usage of zend_hash_release, I'd actually also replace the remaining places where zend_hash_destroy/HASH_RELEASE is used as it considers the refcount/immutability (not present in this case, but who knows in the future) and automatically frees the hashtable struct as well, though probably it would be better to do this in a separate PR...

[alcaeus]

@danog the windows failure is unrelated, and the first impression from Evergreen is that the memory leak is no more. As for the remaining zend_hash_destroy usage, I have no objection to changing those as well, but a separate PR might indeed be the way to go for this. Please be aware that @jmikola is at Laracon these days, so it might be a few days until we can review and merge this PR.