Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.22] oci: fix segfault in pod stop code #5803

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/oci/container.go
Expand Up @@ -553,7 +553,10 @@ func (c *Container) Spoofed() bool {
// If a stop is currently happening, it also sends the new timeout
// along the stopTimeoutChan, allowing the in-progress stop
// to stop faster, or ignore the new stop timeout.
func (c *Container) SetAsStopping(timeout int64) {
// In this case, it also returns true, signifying the caller doesn't have to
// Do any stop related cleanup, as the original caller (alreadyStopping=false)
// will do said cleanup.
func (c *Container) SetAsStopping(timeout int64) (alreadyStopping bool) {
// First, need to check if the container is already stopping
c.stopLock.Lock()
defer c.stopLock.Unlock()
Expand All @@ -567,12 +570,13 @@ func (c *Container) SetAsStopping(timeout int64) {
case <-c.stoppedChan: // This case is to avoid waiting forever once another routine has finished.
case <-c.stopStoppingChan: // This case is to avoid deadlocking with SetAsNotStopping.
}
return
return true
}
// Regardless, set the container as actively stopping.
c.stopping = true
// And reset the stopStoppingChan
c.stopStoppingChan = make(chan struct{}, 1)
return false
}

// SetAsNotStopping unsets the stopping field indicating to new callers that the container
Expand Down
4 changes: 3 additions & 1 deletion internal/oci/runtime_oci.go
Expand Up @@ -692,7 +692,9 @@ func WaitContainerStop(ctx context.Context, c *Container, timeout time.Duration,

// StopContainer stops a container. Timeout is given in seconds.
func (r *runtimeOCI) StopContainer(ctx context.Context, c *Container, timeout int64) (retErr error) {
c.SetAsStopping(timeout)
if c.SetAsStopping(timeout) {
return nil
}
defer func() {
if retErr != nil {
// Failed to stop, set stopping to false.
Expand Down
13 changes: 13 additions & 0 deletions test/ctr.bats
Expand Up @@ -926,3 +926,16 @@ function check_oci_annotation() {
pod_id=$(crictl runp "$TESTDATA"/sandbox_config.json)
! crictl create "$pod_id" "$TESTDIR/config" "$TESTDATA"/sandbox_config.json
}

@test "ctr stop timeouts should decrease" {
start_crio
jq ' .command'='["/bin/sh", "-c", "trap \"echo hi\" INT; /bin/sleep 6000"]' \
"$TESTDATA"/container_config.json > "$newconfig"

ctr_id=$(crictl run "$newconfig" "$TESTDATA"/sandbox_config.json)
for i in {150..1}; do
crictl stop --timeout "$i" "$ctr_id" &
sleep .1
done
crictl stop "$ctr_id"
}