Skip to content

Fix SplObjectStorage getHash() guard leaking across a bailout#82

Closed
iliaal wants to merge 1 commit into
masterfrom
fix/spl-observer-gethash-bailout
Closed

Fix SplObjectStorage getHash() guard leaking across a bailout#82
iliaal wants to merge 1 commit into
masterfrom
fix/spl-observer-gethash-bailout

Conversation

@iliaal

@iliaal iliaal commented Jun 14, 2026

Copy link
Copy Markdown
Owner

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

The getHash() recursion guard increments a request-persistent counter
around the userland getHash() call but decrements it only on the normal
return path. A bailout inside an overridden getHash() (out-of-memory,
timeout, or any fatal) skips the decrement, and the counter is never
reset 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 path.
@iliaal

iliaal commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

Submitted upstream as php#22308.

@iliaal iliaal closed this Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant