Skip to content

Commit

Permalink
oci: fix segfault in pod stop code
Browse files Browse the repository at this point in the history
by only closing the stopStoppingChan once

Signed-off-by: Peter Hunt <pehunt@redhat.com>
  • Loading branch information
haircommander committed Apr 14, 2022
1 parent 28c5a70 commit 25772cb
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
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"
}

0 comments on commit 25772cb

Please sign in to comment.