ext/session: fix session GC fork#22164
Conversation
TimWolla
left a comment
There was a problem hiding this comment.
This is not a correct fix. Invoking the CSPRNG for every GC attempt is needlessly expensive. It is also fallible - if it fails, sessions will never be collected (e.g. when the getrandom() syscall and /dev/urandom are both unavailable).
|
Sharing the sequence across workers is not great, but also not an issue. We are not interested in unpredictable randomness, but instead use the randomness as a proxy for a probability. Different FPM workers will naturally handle different numbers of requests over time, which means that they will be sitting at different points in the sequence and still avoid "hotspots". |
What's the point of randomness then and specifying
Would the acceptable trade-off be to lazy-load it at first request instead of in module initalization? |
I'm not sure what you mean by “not respected”. Even if the sequence is not unpredictable, it will run a collection for gc_probability out of every gc_divisor requests on average.
Probably, yes. |
That's not always given. There may be cases for short-lived processes when GC is never run based on the set GC session configuration. Of course this is edge-case, but theoretically may happen and it's serious bug then. I will try to develop better solution tomorrow. |
|
As a proper long-term fix we probably need per-extension hooks in |
Fixes #21314
If I understand the logic of the bug correctly (retrieved with the help of AI):
globals_ctor(GINIT) runs at module registration inside php_module_startup.fpm_run()forks the workers.PHP_RINIT_FUNCTION(session)never touchesPS(random).pm.max_requests) and respawn, each new worker forks from the master and restarts at position 0 of the identical sequence.Changing the logic back to drawing per request gives back the true randomness.