feat: Add modules to reload as a global variable

[ptondereau]

WIP

We're adding a way to make modules to reload available outside the FrankenPHP (for other extension)

TODO

• Test with a dummy extension
• Add documentation

[ptondereau]

@dunglas this is not finished yet 😅 and it doesn't work for now but thanks for the early review. I'll remove the draft flag when it's finished. Sorry for the noise

[ptondereau]

@dunglas I've made a small PoC not published yet to be able to add module to reload with an environment variable. It works smoothly and solves partially the problem that some extension need to rely on the old PHP request lifecycle to trigger some action (like PHP_RINIT_FUNCTION):
At the startup, it triggers PHP_RINIT_FUNCTION x times when x = number of workers, and then only 1 per HTTP request handled.
To solve this problem, this module to reload declaration should also live in the Go section (worker.go) for only worker mode.
The final goal of all that setup is to use, for example, well-known Symfony Runtimes in worker mode without any modification.
I'm thinking a lot about continuous profiling extension: you need to create a custom Runtime class if you want to add profiling request only behavior.
TBH, I agree that is not a huge deal/impact here, but I would like to know your thoughts.

[Stargateur]

need to handle malloc error, at least return a bool in init_modules_to_reload and handle it where you can like in frankenphp_register_module_to_reload

[Stargateur]

BTW, I was thinking in the shower, and ultimately you could just avoid allocation, so far nobody needed this feature so I doubt there is a need for a lot of space, you could just use a 32 size array and say max is 32. And eventually if people really need more there will ask for it. You could even go for 256 it's not a lot of space for the binary anyway.

Also, all your code is NOT thread safe, I don't know if it's matter or not for this context.

[ptondereau]

BTW, I was thinking in the shower, and ultimately you could just avoid allocation, so far nobody needed this feature so I doubt there is a need for a lot of space, you could just use a 32 size array and say max is 32. And eventually if people really need more there will ask for it. You could even go for 256 it's not a lot of space for the binary anyway.

Also, all your code is NOT thread safe, I don't know if it's matter or not for this context.

I agree with a fixed size.

For the thread's safety, we can declare the struct as a thread local variable but I don't know if it is relevant here because module init (MINIT) is only ran once here

[dunglas]

Couldn't this be implemented directly in the extension by "decorating" (wrapping) the frankenphp_handle_request() function to trigger your cleanup code?

[dunglas]

@realFlowControl found a way to achieve this for a similar use case without having to add code: DataDog/dd-trace-php#3169

Would that fit your needs?

[ptondereau]

@realFlowControl found a way to achieve this for a similar use case without having to add code: DataDog/dd-trace-php#3169

Would that fit your needs?

Thanks for this PR! Indeed, it fits our needs here, but the sapi_activate call happens earlier when $_SERVER is not populated and we need to have access to the request context