Worker script itself is not shutdown gracefully, but hangs #1226
Labels
bug
Something isn't working
Comments
|
Hey thanks for the reproducer! Looks like a thread might not be properly shutting down, I'll have a look when I have time |
|
Are you using the latest FrankenPHP version (1.3.3)? I didn't really see the same behavior with your minimal reproducer. it will wait for the long request to finish and then shut down, as expected. |
|
Thanks for looking into this. I did test with 1.3.1, but just bumped to 1.3.3 and same result.
This still yields the same result when stop the process via SIGTERM (via docker compose stop) during the slow request: Debug logs, note: num_threads=2, and worker.num=1 The debug output does not contain this though: if c := logger.Check(zapcore.DebugLevel, "shutting down"); c != nil {
c.Write(zap.String("worker", thread.worker.fileName))
}nor this: logger.Debug("FrankenPHP shut down" |
|
Particularly, this part: Seems to highlight that the worker is not shutdown? Because there is a new |
|
My caddyfile in docker (mostly irrelevant stuff) {
# The server only runs on port 80 behind a reverse proxy.
# We don't https.
auto_https off
# No reason to add certificates in local dev.
# Caddy only runs in docker.
skip_install_trust
log {
level {$CADDY_GLOBAL_LOG_LEVEL}
output stderr
}
frankenphp {
# num_threads 2 is inserted here.
{$CADDY_FRANKENPHP_OPTIONS}
worker {
file public/frankenphp-worker.php
{$CADDY_FRANKENPHP_WORKER_WATCH_OPTIONS}
# num 1 is inserted here.
{$CADDY_FRANKENPHP_WORKER_NUM_THREADS}
}
}
}
http://:80 {
log {
level {$CADDY_SERVER_LOG_LEVEL}
output stderr
}
route {
root public
# Perform compression here, so that we need to send less data to the reverse proxy.
encode zstd br gzip
php_server {
index frankenphp-worker.php
# Required for the public/storage/ directory...
resolve_root_symlink
}
}
} |
|
Ah I think now I get it. It looks like this was always the case, no matter if you are shutting down during a long request or not. @dunglas is there a reason why the graceful shutdown is not triggered when stopping the process via the admin API? In other words: why is Lines 115 to 122 in fc6be5d |
|
Yeah, you don't need the slow request, that's just to show that in-flight requests shut down gracefully, while the script does not. Regarding shutdown, it seems that it's only called in a destructor in the main frankenphp file. Line 54 in fc6be5d |
Probably just an oversight or potential changes in caddy since it was originally written. We should add a test for this. |
|
This probably never worker properly. The |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What happened?
During graceful shutdown of FrankenPHP via SIGTERM or admin API, in-flight requests complete gracefully, but the worker script itself does not.
This means that it's impossible to perform cleanup tasks after a worker is stopped (for example, sending telemetry data)
Minimal Reproducer
Current Behaviour
When initiating a graceful shutdown during a slow request, the log file might show this:
Expected Behaviour
The log output should be:
And only once that script exists, the pthread is killed.
Best guess on why
Maybe there is a race condition, starting here:
When
workersDoneis closed, this select ingo_frankenphp_worker_handle_request_startshould trigger:and return
falseto thefrankenphp_handle_request_functionwhich breaks the loop.
But, I never see
"shutting down"in the server output (debug enabled), nor do I seeFrankenPHP shut down.This might be a separate problem, maybe because shutdown() runs as a destructor (idk?)
So either the above code never runs, or if it does run, PHP does not finish fast enough before this returns false (caused by
drainThreads)The
return falsecauses this C code to callts_free_threadBuild Type
Docker (Debian Bookworm)
Worker Mode
Yes
Operating System
GNU/Linux
CPU Architecture
x86_64
PHP configuration
Relevant log output
No response
The text was updated successfully, but these errors were encountered: