Clarification on refcounting behavior of object handlers/get_gc

[danog]

Description

When implementing mongodb/mongo-php-driver#1850, I have encountered some strange refcounting behavior within the garbage collector; specifically, when working with the hashtable returned by get_gc in https://github.com/php/php-src/blob/master/Zend/zend_gc.c#L892 and https://github.com/php/php-src/blob/master/Zend/zend_gc.c#L1516 the refcount is incremented and then seemingly not decremented again (leading to the leaks I'm fighting in mongodb/mongo-php-driver#1850), but in https://github.com/php/php-src/blob/master/Zend/zend_gc.c#L1068, the refcount is decremented correctly.

This brings the question: what should be the refcount of the hashtables returned by get_gc, get_properties_for (in all of the various modes), get_properties, get_debug_info, and all other object handlers?

This detail is not documented anywhere, and specifically for get_gc, it's especially not clear given that returning a refcount of 0 would cause a negative refcount in one place, and returning 1 causes a leak in other two places (though that may be due to an issue in my implementation in the mongodb/mongo-php-driver#1850 PR, though it's not immediately obvious what it is to me).

It would be super nice to have it documented at least inside of the source code as comments too...

PHP Version

8.4.10

Operating System

No response

[bwoebi]

The HashTable returned by get_gc is not refcounted at all. If you need to return a HashTable from get_gc, you need to make it either outlive the current gc run or tie it to the lifetime of the object. (if you need dynamic data to be passed as gc, it should be put into the zend_get_gc_buffer_* stuff.)
Also I really cannot recommend ever passing stuff with a RC=0 around, that's just a big possible source of bugs.

All other handlers must return a incref'ed value which the caller can release.

[iluuu1994]

specifically, when working with the hashtable returned by get_gc in https://github.com/php/php-src/blob/master/Zend/zend_gc.c#L892 and https://github.com/php/php-src/blob/master/Zend/zend_gc.c#L1516 the refcount is incremented and then seemingly not decremented again

This is part of how the GC works. It first, all nodes in the graph are traversed, the refcount of the node is decreased, so are all of its references. Afterwards, all roots of cycles should have refcount 0 if they are not referenced anywhere outside of the cycle. That's what the deref does, it's not here to free the memory, but to understand whether the object is involved in a cycle. Same with the addref, it restores the refcount once we're done with the analysis and know what to free.

As Bob said, the HashTable returned from get_gc should not have an additional added ref. It's primarily used for zend_object.properties, which is itself part of the cycle.

Does this help?

[danog]

I believe this answers my question, and indeed returning the hashtable from get_gc without incrementing the refcount (unlike for get_properties) makes everything behave properly.

These explanations should probably be added somewhere in the docs :)

[iluuu1994]

These explanations should probably be added somewhere in the docs :)

If you wish we can keep this open until then. There's a small description in phpinternalsbook ¹ but it's not very precise:

The get_gc handler should return all variables that are held by the object, so cyclic dependencies can be properly collected. If the object doesn’t maintain a property hashmap (because it doesn’t store any dynamic properties) it can use table to store a pointer directly into the list of zvals, along with a count of properties.

It does hint at the fact that the property hashmap is owned by the object, but wouldn't hurt to spell it out.

Footnotes

1. https://www.phpinternalsbook.com/php7/classes_objects/object_handlers.html#c.get_gc ↩