You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While the helper process will eventually timeout if the application is finalised, it will probably not do so after a restart, as
the timeout is not intended to cover these scenarios. This might prevent a successful upgrade, although this will be mitigated in the future through #3 and #4.
Suggested solution
Signalling the helper process to terminate during MSHUTDOWN should suffice, although the helper process PID is not available to the extension at the moment, but we can obtain this through the SCM_CREDENTIALS on client_init.
However, if the process performing MSHUTDOWN does not partake in the request lifecycle it will never really have a client_init, so a better approach is to use fcntl locks instead of flock, which would allow us to get the PID of the helper process by inspecting struct flock, but since fcntl locks are not inherited after fork, it'll require a few more changes.
Another slightly more complex solution is to implement a new command which would allow the extension to tell the helper process to shutdown. I'm not sure this is a good solution in general as it'll allow an unverified process to terminate the helper with a limited (but equivalent) set of credentials.
An even simpler and quite common solution is to have the final process doing fork/exec actually write it's PID in the lock file. This would allow the process performing MSHUTDOWN to try-lock and if it fails, it can read the PID and send a signal.
The text was updated successfully, but these errors were encountered:
I don't think that this is a workable change, nor that the problem it's trying to solve (the helper outliving the clients) is a big problem. First, the helper will be closed in two common scenarios:
inside docker and,
when fpm/apache run inside a cgroup (e.g. when fpm/apache are run through a systemd service).
In these two common scenarios, child processes (even if they daemonize) are tracked and closed when the service ends. Additionally, if the helper process is part of another systemd service, we do not want to make it exit. So we should exit only if we launched the helper. The problem is that, in general, we can't rely on MSHUTDOWN for this. First off, we have no guarantee that MSHUTDOWN will be called when a client exits (it could crash or be be killed for instance). More importantly, clients shutdown as a matter of course, for instance when they've served the maximum number of requests. We would then need to keep a counter for the active clients and shutdown when this reaches 0. This is in itself a bit complicated to make race-free, but even then it's not clear to me how this can work in low traffic scenarios in which possibly only one client exists at one time.
The upgrade/downgrade problem can be mitigated, as you said, with the versioning of the lock/socket. We can also be more aggressive with inactivity timeout.
Description
While the helper process will eventually timeout if the application is finalised, it will probably not do so after a restart, as
the timeout is not intended to cover these scenarios. This might prevent a successful upgrade, although this will be mitigated in the future through #3 and #4.
Suggested solution
MSHUTDOWN
should suffice, although the helper process PID is not available to the extension at the moment, but we can obtain this through theSCM_CREDENTIALS
onclient_init
.MSHUTDOWN
does not partake in the request lifecycle it will never really have aclient_init
, so a better approach is to usefcntl
locks instead offlock
, which would allow us to get the PID of the helper process by inspectingstruct flock
, but sincefcntl
locks are not inherited afterfork
, it'll require a few more changes.fork
/exec
actually write it's PID in the lock file. This would allow the process performingMSHUTDOWN
to try-lock and if it fails, it can read the PID and send a signal.The text was updated successfully, but these errors were encountered: