Skip to content

PHPC-2505: Fix foreach after GC invocation #1940

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

Draft
jmikola wants to merge 1 commit into
base: v2.x
Choose a base branch
from
Draft

PHPC-2505: Fix foreach after GC invocation #1940

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
10 changes: 0 additions & 10 deletions php_phongo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,6 @@ static zend_class_entry* php_phongo_fetch_internal_class(const char* class_name,
return NULL;
}

static HashTable* php_phongo_std_get_gc(zend_object* object, zval** table, int* n)
{
*table = NULL;
*n = 0;
return zend_std_get_properties(object);
}

PHP_MINIT_FUNCTION(mongodb) /* {{{ */
{
bson_mem_vtable_t bson_mem_vtable = {
Expand Down Expand Up @@ -197,9 +190,6 @@ PHP_MINIT_FUNCTION(mongodb) /* {{{ */
/* Disable cloning by default. Individual classes can opt in if they need to
* support this (e.g. BSON objects). */
phongo_std_object_handlers.clone_obj = NULL;
/* Ensure that get_gc delegates to zend_std_get_properties directly in case
* our class defines a get_properties handler for debugging purposes. */
phongo_std_object_handlers.get_gc = php_phongo_std_get_gc;

/* Initialize zend_class_entry dependencies.
*
Expand Down
99 changes: 97 additions & 2 deletions php_phongo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,106 @@ zend_object_handlers* phongo_get_std_object_handlers(void);
#define PHONGO_GET_PROPERTY_HASH_FREE_PROPS(is_temp, props) \
do { \
if (is_temp) { \
zend_hash_destroy((props)); \
FREE_HASHTABLE(props); \
zend_hash_release((props)); \
} \
} while (0)

#define PHONGO_ASSIGN_PROPERTY_HANDLERS(_name) \
do { \
php_phongo_handler_##_name.read_property = php_phongo_##_name##_read_property; \
php_phongo_handler_##_name.write_property = php_phongo_##_name##_write_property; \
php_phongo_handler_##_name.has_property = php_phongo_##_name##_has_property; \
php_phongo_handler_##_name.unset_property = php_phongo_##_name##_unset_property; \
php_phongo_handler_##_name.get_property_ptr_ptr = php_phongo_##_name##_get_property_ptr_ptr; \
php_phongo_handler_##_name.get_gc = php_phongo_##_name##_get_gc; \
} while (0)

#define PHONGO_DEFINE_PROPERTY_HANDLERS(_name, _intern_extractor) \
static zval* php_phongo_##_name##_read_property(zend_object* zobj, zend_string* member, int type, void** cache_slot, zval* rv) \
{ \
HashTable* props = _intern_extractor(zobj)->php_properties; \
if (!props) { \
ALLOC_HASHTABLE(props); \
zend_hash_init(props, 0, NULL, ZVAL_PTR_DTOR, 0); \
_intern_extractor(zobj)->php_properties = props; \
} \
zval* ret = zend_hash_find(props, member); \
if (ret) { \
return ret; \
} \
return &EG(uninitialized_zval); \
} \
\
static zval* php_phongo_##_name##_write_property(zend_object* zobj, zend_string* name, zval* value, void** cache_slot) \
{ \
Z_TRY_ADDREF_P(value); \
HashTable* props = _intern_extractor(zobj)->php_properties; \
if (!props) { \
ALLOC_HASHTABLE(props); \
zend_hash_init(props, 0, NULL, ZVAL_PTR_DTOR, 0); \
_intern_extractor(zobj)->php_properties = props; \
} \
return zend_hash_add_new(props, name, value); \
} \
\
static int php_phongo_##_name##_has_property(zend_object* zobj, zend_string* name, int has_set_exists, void** cache_slot) \
{ \
HashTable* props = _intern_extractor(zobj)->php_properties; \
if (!props) { \
ALLOC_HASHTABLE(props); \
zend_hash_init(props, 0, NULL, ZVAL_PTR_DTOR, 0); \
_intern_extractor(zobj)->php_properties = props; \
} \
zval* value = zend_hash_find(props, name); \
if (value) { \
if (has_set_exists == ZEND_PROPERTY_NOT_EMPTY) { \
return zend_is_true(value); \
} \
if (has_set_exists < ZEND_PROPERTY_NOT_EMPTY) { \
ZEND_ASSERT(has_set_exists == ZEND_PROPERTY_ISSET); \
ZVAL_DEREF(value); \
return (Z_TYPE_P(value) != IS_NULL); \
} \
ZEND_ASSERT(has_set_exists == ZEND_PROPERTY_EXISTS); \
return true; \
} \
return false; \
} \
\
static void php_phongo_##_name##_unset_property(zend_object* zobj, zend_string* name, void** cache_slot) \
{ \
HashTable* props = _intern_extractor(zobj)->php_properties; \
if (!props) { \
ALLOC_HASHTABLE(props); \
zend_hash_init(props, 0, NULL, ZVAL_PTR_DTOR, 0); \
_intern_extractor(zobj)->php_properties = props; \
} \
zend_hash_del(props, name); \
} \
\
static zval* php_phongo_##_name##_get_property_ptr_ptr(zend_object* zobj, zend_string* name, int type, void** cache_slot) \
{ \
HashTable* props = _intern_extractor(zobj)->php_properties; \
if (!props) { \
ALLOC_HASHTABLE(props); \
zend_hash_init(props, 0, NULL, ZVAL_PTR_DTOR, 0); \
_intern_extractor(zobj)->php_properties = props; \
} \
\
zval* value = zend_hash_find(props, name); \
if (value) { \
return value; \
} \
return zend_hash_add(props, name, &EG(uninitialized_zval)); \
} \
\
static HashTable* php_phongo_##_name##_get_gc(zend_object* zobj, zval** table, int* n) \
{ \
*table = NULL; \
*n = 0; \
return _intern_extractor(zobj)->php_properties; \
}

#define PHONGO_ZVAL_EXCEPTION_NAME(e) (ZSTR_VAL(e->ce->name))

#define PHONGO_SET_CREATED_BY_PID(intern) \
Expand Down
11 changes: 9 additions & 2 deletions src/BSON/Binary.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,11 @@ static void php_phongo_binary_free_object(zend_object* object)
}

if (intern->properties) {
zend_hash_destroy(intern->properties);
FREE_HASHTABLE(intern->properties);
zend_hash_release(intern->properties);
}

if (intern->php_properties) {
zend_hash_release(intern->php_properties);
}
Comment on lines 227 to 233
Copy link
Member

@alcaeus alcaeus Feb 10, 2026

Choose a reason for hiding this comment

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

Refactoring suggestion: we could extract this to a PHONGO_RELEASE_INTERN_PROPERTIES macro as this code is the same in all classes. Feel free to defer this to a follow-up ticket.

Copy link
Contributor Author

@jmikola jmikola Feb 10, 2026

Choose a reason for hiding this comment

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

I did originally have a PHONGO_FREE_PROPS macro but removed it before finalizing the commit because I felt it made the free_object handler a bit less clear. And I didn't like it was selectively used only in classes that defined both properties and php_properties on their internal struct. Happy to add that back, though, as it would remove duplication.

}

Expand Down Expand Up @@ -320,6 +323,8 @@ static HashTable* php_phongo_binary_get_properties(zend_object* object)
return php_phongo_binary_get_properties_hash(object, false);
}

PHONGO_DEFINE_PROPERTY_HANDLERS(binary, Z_OBJ_BINARY);

void php_phongo_binary_init_ce(INIT_FUNC_ARGS)
{
php_phongo_binary_ce = register_class_MongoDB_BSON_Binary(php_phongo_binary_interface_ce, php_phongo_json_serializable_ce, php_phongo_type_ce, zend_ce_stringable);
Expand All @@ -332,6 +337,8 @@ void php_phongo_binary_init_ce(INIT_FUNC_ARGS)
php_phongo_handler_binary.get_properties = php_phongo_binary_get_properties;
php_phongo_handler_binary.free_obj = php_phongo_binary_free_object;
php_phongo_handler_binary.offset = XtOffsetOf(php_phongo_binary_t, std);

PHONGO_ASSIGN_PROPERTY_HANDLERS(binary);
}

bool phongo_binary_new(zval* object, const char* data, size_t data_len, bson_subtype_t type)
Expand Down
11 changes: 9 additions & 2 deletions src/BSON/DBPointer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,11 @@ static void php_phongo_dbpointer_free_object(zend_object* object)
}

if (intern->properties) {
zend_hash_destroy(intern->properties);
FREE_HASHTABLE(intern->properties);
zend_hash_release(intern->properties);
}

if (intern->php_properties) {
zend_hash_release(intern->php_properties);
}
}

Expand Down Expand Up @@ -241,6 +244,8 @@ static HashTable* php_phongo_dbpointer_get_properties(zend_object* object)
return php_phongo_dbpointer_get_properties_hash(object, false);
}

PHONGO_DEFINE_PROPERTY_HANDLERS(dbpointer, Z_OBJ_DBPOINTER);

void php_phongo_dbpointer_init_ce(INIT_FUNC_ARGS)
{
php_phongo_dbpointer_ce = register_class_MongoDB_BSON_DBPointer(php_phongo_json_serializable_ce, php_phongo_type_ce, zend_ce_stringable);
Expand All @@ -253,6 +258,8 @@ void php_phongo_dbpointer_init_ce(INIT_FUNC_ARGS)
php_phongo_handler_dbpointer.get_properties = php_phongo_dbpointer_get_properties;
php_phongo_handler_dbpointer.free_obj = php_phongo_dbpointer_free_object;
php_phongo_handler_dbpointer.offset = XtOffsetOf(php_phongo_dbpointer_t, std);

PHONGO_ASSIGN_PROPERTY_HANDLERS(dbpointer);
}

bool phongo_dbpointer_new(zval* object, const char* ref, size_t ref_len, const bson_oid_t* oid)
Expand Down
11 changes: 9 additions & 2 deletions src/BSON/Decimal128.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,11 @@ static void php_phongo_decimal128_free_object(zend_object* object)
zend_object_std_dtor(&intern->std);

if (intern->properties) {
zend_hash_destroy(intern->properties);
FREE_HASHTABLE(intern->properties);
zend_hash_release(intern->properties);
}

if (intern->php_properties) {
zend_hash_release(intern->php_properties);
}
}

Expand Down Expand Up @@ -216,6 +219,8 @@ static HashTable* php_phongo_decimal128_get_properties(zend_object* object)
return php_phongo_decimal128_get_properties_hash(object, false);
}

PHONGO_DEFINE_PROPERTY_HANDLERS(decimal128, Z_OBJ_DECIMAL128)

void php_phongo_decimal128_init_ce(INIT_FUNC_ARGS)
{
php_phongo_decimal128_ce = register_class_MongoDB_BSON_Decimal128(php_phongo_decimal128_interface_ce, php_phongo_json_serializable_ce, php_phongo_type_ce, zend_ce_stringable);
Expand All @@ -227,6 +232,8 @@ void php_phongo_decimal128_init_ce(INIT_FUNC_ARGS)
php_phongo_handler_decimal128.get_properties = php_phongo_decimal128_get_properties;
php_phongo_handler_decimal128.free_obj = php_phongo_decimal128_free_object;
php_phongo_handler_decimal128.offset = XtOffsetOf(php_phongo_decimal128_t, std);

PHONGO_ASSIGN_PROPERTY_HANDLERS(decimal128);
}

bool phongo_decimal128_new(zval* object, const bson_decimal128_t* decimal)
Expand Down
3 changes: 1 addition & 2 deletions src/BSON/Document.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,8 +447,7 @@ static void php_phongo_document_free_object(zend_object* object)
}

if (intern->properties) {
zend_hash_destroy(intern->properties);
FREE_HASHTABLE(intern->properties);
zend_hash_release(intern->properties);
}
}

Expand Down
11 changes: 9 additions & 2 deletions src/BSON/Int64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ static void php_phongo_int64_free_object(zend_object* object)
zend_object_std_dtor(&intern->std);

if (intern->properties) {
zend_hash_destroy(intern->properties);
FREE_HASHTABLE(intern->properties);
zend_hash_release(intern->properties);
}

if (intern->php_properties) {
zend_hash_release(intern->php_properties);
}
}

Expand Down Expand Up @@ -554,6 +557,8 @@ static HashTable* php_phongo_int64_get_properties(zend_object* object)
return php_phongo_int64_get_properties_hash(object, false);
}

PHONGO_DEFINE_PROPERTY_HANDLERS(int64, Z_OBJ_INT64)

void php_phongo_int64_init_ce(INIT_FUNC_ARGS)
{
php_phongo_int64_ce = register_class_MongoDB_BSON_Int64(php_phongo_json_serializable_ce, php_phongo_type_ce, zend_ce_stringable);
Expand All @@ -568,6 +573,8 @@ void php_phongo_int64_init_ce(INIT_FUNC_ARGS)
php_phongo_handler_int64.offset = XtOffsetOf(php_phongo_int64_t, std);
php_phongo_handler_int64.cast_object = php_phongo_int64_cast_object;
php_phongo_handler_int64.do_operation = php_phongo_int64_do_operation;

PHONGO_ASSIGN_PROPERTY_HANDLERS(int64);
}

bool phongo_int64_new(zval* object, int64_t integer)
Expand Down
11 changes: 9 additions & 2 deletions src/BSON/Iterator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,11 @@ static void php_phongo_iterator_free_object(zend_object* object)
zend_object_std_dtor(&intern->std);

if (intern->properties) {
zend_hash_destroy(intern->properties);
FREE_HASHTABLE(intern->properties);
zend_hash_release(intern->properties);
}

if (intern->php_properties) {
zend_hash_release(intern->php_properties);
}

php_phongo_iterator_free_current(intern);
Expand Down Expand Up @@ -378,6 +381,8 @@ static zend_object_iterator* php_phongo_iterator_get_iterator(zend_class_entry*
return iterator;
}

PHONGO_DEFINE_PROPERTY_HANDLERS(iterator, Z_OBJ_ITERATOR)

void php_phongo_iterator_init_ce(INIT_FUNC_ARGS)
{
php_phongo_iterator_ce = register_class_MongoDB_BSON_Iterator(zend_ce_iterator);
Expand All @@ -390,4 +395,6 @@ void php_phongo_iterator_init_ce(INIT_FUNC_ARGS)
php_phongo_handler_iterator.get_properties = php_phongo_iterator_get_properties;
php_phongo_handler_iterator.free_obj = php_phongo_iterator_free_object;
php_phongo_handler_iterator.offset = XtOffsetOf(php_phongo_iterator_t, std);

PHONGO_ASSIGN_PROPERTY_HANDLERS(iterator);
}
Loading
Loading