Fix SplObjectStorage getHash() guard leaking across a bailout

[iliaal]

The getHash() recursion guard bumps a request-persistent counter around the userland getHash() call but only decrements it on the normal return. A bailout inside an overridden getHash() (out of memory, timeout, any fatal) skips the decrement, and nothing resets the counter per request, so on a persistent SAPI every later request on the same worker wrongly throws "Modification of SplObjectStorage during getHash() is prohibited". Decrement inside a zend_catch and re-raise the bailout so the counter is balanced on every exit.

class P extends SplObjectStorage {
    public function getHash($o): string {
        ini_set('memory_limit', '2M');
        str_repeat('a', 100 * 1024 * 1024); // OOM bailout
        return 'x';
    }
}
(new P())->offsetSet(new stdClass());           // request 1 bails out
(new SplObjectStorage())->offsetSet(new stdClass()); // later request, no override: throws before the patch

[ndossche]

A much nicer way is likely to reset spl_object_storage_get_hash_depth on request init/shutdown.

[iliaal]

Agreed, cleaner. Reset the counter in SPL's existing RINIT and dropped the zend_try/catch. Folded into PHP_RINIT(spl) rather than a new per-extension hook, and RINIT over RSHUTDOWN so a worker that skipped a clean shutdown still starts at zero.

[ndossche]

You need a specific skipif include for the usage of php_cli_server_start in your test

[iliaal]

Moved it to ext/spl/tests/ and added the php_cli_server.inc availability guard to SKIPIF.

[ndossche]

I think this is also okay to go into 8.4.

[iliaal]

I think this is also okay to go into 8.4.

8.4 and 8.5 don't have the getHash() recursion guard at all, it was added on master in d694135 (GH-21831). No counter there to leak across a bailout, so nothing to backport.

[ndossche]

Ah okay, apologies. Good to go then