Repository navigation
Revert "core/service: when resetting PID also reset known flag" - #28000
Conversation
This reverts commit ff32060. This change is incorrect as we don't want to mark the PID as invalid but only mark it as dead. The change in question also breaks user level socket activation for `podman.service` as the termination of the main `podman system service` process is not properly handled, causing any application accessing the socket to hang. This is because the user-level `podman.service` unit also hosts two non-main processes: `rootlessport` and `rootlessport-child` which causes the `cgroup_good` check to still succeed. The original submitter of this commit is recommended to find another more correct way to fix the cgroupsv1 issue on CentOS 8.
github-actions
Bot
added
the
please-review
|
cc @msekletar: submitter of mentioned change |
|
Edit: I need to read better :/ |
|
Hmm, this looks very wrong to me. Please elaborate why precisely this is suppsoed to fix. If the main process of service goes down a service should be considered dead, except if RemainAftreExit= is set. |
Well, that's what this PR is supposed to achieve. The commit being reverted is what's breaking this. |
|
More details here: redhat-plumbers/systemd-rhel8#391 (comment) |
|
So i think this revert PR actually is the right thing to do. There's a difference between "Hey, the main pid was known and is dead now" and "Hey, i don't know the main pid". And this matters for the state machine, for example the key part of main_pid_good() explicitly checks for this. hence, let's revert this, the original patch wasn't right. sorry for taking so long to deep dive into this. |
github-actions
Bot
removed
the
please-review
|
Sounds like this also closes #28030. @poettering It would be good to backport this. |
THe original commit was never in a released version. Hence no point in backporting that. Also, systemd-stable tree is maintained in its own repo, and I am not involved. |
FTR, there's already an open request for such backport there: systemd/systemd-stable#302 |
It was in 253.5: ae83e97
I wasn't aware you weren't involved in systemd-stable. Anyway, I'm fairly new to systemd release processes, but I kinda thought that the backport process happened by leaving comments on PRs requesting for them to be backported, leading to a label being added, leading to a backport being included in the next point release. If there's a better process for this, I'd be happy to know; it's not in the contribution guidelines |
Yes, that's how it's being done so far - backports are requested, labeled, and eventually put together into a batch and a point release in the stable repo. |
|
@bluca This one also needs backporting into v252-stable. It's a fairly serious bug that causes services to stay in active state despite the mainpid terminating. See also systemd/systemd-stable#302. I meant to reportbug this against Debian testing last week, but then I noticed unstable is already on v253 which doesn't have the buggy patch yet, and v253.6 will have both the patch and its revert, so it felt like a waste of time… BUT then you uploaded 252.11 to Debian stable, and that's a bit of a disaster. In retrospect I should've done the reportbug earlier to warn you. Apologies! |
This reverts commit ff32060.
This change is incorrect as we don't want to mark the PID as invalid but only mark it as dead so that
main_pid_goodwould return false instead of-EAGAIN.The change in question also breaks user level socket activation for
podman.serviceas the termination of the mainpodman system serviceprocess is not properly handled, causing any application accessing the socket to hang.This is because the user-level
podman.serviceunit also hosts two non-main processes:rootlessportandrootlessport-childwhich causes thecgroup_goodcheck to still succeed.The original submitter of this commit is recommended to find another more correct way to fix the cgroupsv1 issue on CentOS 8.
Fixes #27953