libpod: remove UpdateContainerStatus()

[Luap99]

There are two major problems with UpdateContainerStatus() First, it can deadlock when the the state json is to big as it tries to read stderr until EOF but it will never hit EOF as long as the runtime process is alive. This means if the runtime json is to big to git into the pipe buffer we deadlock ourselves.
Second, the function modifies the container state struct and even adds and exit code to the db however when it is called from the stop() code path we will be unlocked here.

While the first problem is easy to fix the second one not so much. And when we cannot update the state there is no point in reading the from runtime in the first place as such remove the function as it does more harm then good.

And add some warnings the the functions that might be called unlocked.

Fixes #22246

Does this PR introduce a user-facing change?

Fixed a possible deadlock in the container stop code path when using big annotations 

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

@mheon PTAL

[mheon]

This was dealing with a legitimate bug (podman kill failing because of ESRCH was not updating container status to Exited, thus making the stop code produce bad results on already-dead containers in some circumstances). Can we restore the code, but in the locked part of stop() if an error was returned from killContainer()?

[Luap99]

This was dealing with a legitimate bug (podman kill failing because of ESRCH was not updating container status to Exited, thus making the stop code produce bad results on already-dead containers in some circumstances). Can we restore the code, but in the locked part of stop() if an error was returned from killContainer()?

Should we trust the clean-up process to update the state? I don't follow the logic here. This function was only called when kill failed (i.e. ESRCH) in which case the container process wasn't running so why should podman kill have to do anything in this case besides reporting an error?

[mheon]

The original issue this was attempting to fix is #8086 - sig-proxy errors, apparently?

UpdateContainerStatus definitely feels like the wrong way to resolve those. It feels like a manual kill with signal 0, check for ESRCH, would be more than sufficient to suppress the error?

[Luap99]

@mheon updated to return ErrCtrStateInvalid which is already handled in ProxySignals(), this should prevent #8086 but I haven't tried reproducing this

[mheon]

LGTM on my end. I don't think the original issue is easy to reproduce, so spending that much effort on it probably not a good idea

[rhatdan]

/lgtm