laravel / octane

Supercharge your Laravel application's performance.
https://laravel.com/docs/octane
MIT License
3.76k stars 293 forks source link

Finishes garbage collection before restarting the FrankenPHP worker process #897

Closed AlliBalliBaba closed 4 months ago

AlliBalliBaba commented 4 months ago

I noticed random server crashes when the FrankenPHP worker gracefully restarts. Usually php artisan octane:start will just exit with code 139.

This merge request mitigates these crashes by triggering garbage collection before restarting the server.

FrankenPHP generally recommends in its docs to trigger garbage collection after each request. I don't know if this is necessary, it slows down workers on simple request by ~10% in my tests.

What seems to be necessary though is finishing garbage collection before restarting the FrankenPHP process, otherwise memory can potentially be reused before it is freed by the GC (at least that's what I think happens). This will also not slow down the worker process.

might also be related.

taylorotwell commented 4 months ago

Does this actually fix your issue?

dunglas commented 4 months ago

I don't know if this patch fully fixes the issue, and the bug must be fixed in PHP itself too, but in the meantime we can merge it because at least it cannot cause any harm.

dunglas commented 4 months ago

Regarding garbage collection after each request, the rationale is to prevent the GC to be triggered in the middle of a request. But maybe should we do that after N requests or make the value configurable.