php/php-src — pull request #550 by ahamid (2013-12-24 05:15:11+00:00)
Title: Issue 66085 - discussion

Submitting pull request for discussion only (i.e. please don't merge this ;), see: https://bugs.php.net/bug.php?id=66085

When invoking a custom serializer which itself uses serialize, serialized temporary objects are unreferenced and garbage collected. Their handles and zval memory locations can be reused which results in defeating the reference capability of serialize (the collected objects are viewed as having been visited and are candidates for reference linking.

I don't see any great solutions but here are a few:

1. Just don't support re-entrant serialize with reference linking. Create a new serialization context for every serialize entrance.
1. It turns out (from what I can tell) zvals do not have unique ids over time. Handles are just slots which are reused and memory locations are also reused exactly, so these conspire against being able to distinguish a previous dead object from a live object occupying the same memory location. A solution could be to introduce a monotonic counter which is set on every zval produced in order to produce completely unique ids for the lifetime of the php process. This presumably would entail modifying zval or other core structure so could affect (albiet innocuously) a large surface area). With truly unique ids, a live zval could never be confused with a previous live or dead zval.
1. Forcibly prevent collection of zvals implicated in an object graph undergoing serialization. This can be done by incrementing a reference count, and storing a list of visited zvals so their unreference/collection can be deferred until the end of the top level serialize call.

The last option is what this patch implements.

I do not vouch for this implementation, in specific, I found I needed to `Z_ADDREF_P` when capturing zvals but `zval_ptr_dtor` for cleanup, otherwise I would be warned about leaked objects. I'm not clear why this would be, or if I'm not just lucky here. Working around the GC in this way is at best a hack and at worst possibly really stupid, so I'm just curious for my own sake whether my understanding of the issues here are correct.

For what it is worth, I also investigated msgpack-php implementation, and it appears their impl shares (or is heavily inspired) by the PHP serialize impl (or vice versa), and I wouldn't be surprised if they run into the same issue.