diff --git a/go.mod b/go.mod index d063558784b..2b92d6b50b4 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/coreos/go-systemd/v22 v22.6.0 github.com/creack/pty v1.1.24 github.com/godbus/dbus/v5 v5.1.0 - github.com/gofrs/flock v0.8.0 + github.com/gofrs/flock v0.11.0 github.com/gogo/protobuf v1.3.2 github.com/google/btree v1.1.2 github.com/google/go-github/v84 v84.0.0 diff --git a/go.sum b/go.sum index 7835f843768..821f70a3d7d 100644 --- a/go.sum +++ b/go.sum @@ -116,8 +116,8 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= -github.com/gofrs/flock v0.8.0 h1:MSdYClljsF3PbENUUEx85nkWfJSGfzYI9yEBZOJz6CY= -github.com/gofrs/flock v0.8.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= +github.com/gofrs/flock v0.11.0 h1:AGFQxrpWd8ezw60AvLWIPbxMydNfF8564pwH3FCty0g= +github.com/gofrs/flock v0.11.0/go.mod h1:FirDy1Ing0mI2+kB6wk+vyyAH+e6xiE+EYA0jnzV9jc= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= diff --git a/runsc/container/BUILD b/runsc/container/BUILD index 8296e6c718d..16429e0c026 100644 --- a/runsc/container/BUILD +++ b/runsc/container/BUILD @@ -115,6 +115,12 @@ go_test( ], ) +go_test( + name = "state_file_test", + srcs = ["state_file_test.go"], + embed = [":container"], +) + go_test( name = "serialization_test", srcs = ["serialization_test.go"], diff --git a/runsc/container/state_file.go b/runsc/container/state_file.go index e2d0bd72465..eed0c29055b 100644 --- a/runsc/container/state_file.go +++ b/runsc/container/state_file.go @@ -301,14 +301,14 @@ func (s *StateFile) lock(tryLock TryLock) error { if tryLock { gotLock, err := s.flock.TryLock() if err != nil { - return fmt.Errorf("acquiring lock on %q: %v", s.flock, err) + return fmt.Errorf("acquiring lock on %q: %w", s.flock, err) } if !gotLock { return ErrStateFileLocked } } else { if err := s.flock.Lock(); err != nil { - return fmt.Errorf("acquiring lock on %q: %v", s.flock, err) + return fmt.Errorf("acquiring lock on %q: %w", s.flock, err) } } return nil @@ -387,7 +387,14 @@ func (s *StateFile) Stat() (os.FileInfo, error) { } func (s *StateFile) load(v any, opts LoadOpts) error { + // Open the lock file without O_CREATE, so that loading a destroyed container does not recreate it. + s.once.Do(func() { + s.flock = flock.New(s.lockPath(), flock.SetFlag(os.O_RDONLY)) + }) if err := s.lock(opts.TryLock); err != nil { + if errors.Is(err, os.ErrNotExist) { + return os.ErrNotExist + } return err } defer s.UnlockOrDie() diff --git a/runsc/container/state_file_test.go b/runsc/container/state_file_test.go new file mode 100644 index 00000000000..dbc5736a9fd --- /dev/null +++ b/runsc/container/state_file_test.go @@ -0,0 +1,75 @@ +// Copyright 2026 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package container + +import ( + "os" + "testing" +) + +// TestLoadAfterDestroy runs Load's steps around a concurrent Destroy: the +// state file is found, then destroyed, then locked and read. +func TestLoadAfterDestroy(t *testing.T) { + dir := t.TempDir() + id := FullID{SandboxID: "sandbox", ContainerID: "container"} + + creator := StateFile{RootDir: dir, ID: id} + if err := creator.LockForNew(); err != nil { + t.Fatalf("LockForNew: %v", err) + } + if err := creator.SaveLocked(&Container{}); err != nil { + t.Fatalf("SaveLocked: %v", err) + } + creator.UnlockOrDie() + + found, err := findContainerID(dir, id.ContainerID) + if err != nil { + t.Fatalf("findContainerID: %v", err) + } + + deleter := StateFile{RootDir: dir, ID: id} + if err := deleter.lock(BlockAcquire); err != nil { + t.Fatalf("lock: %v", err) + } + if err := deleter.Destroy(); err != nil { + t.Fatalf("Destroy: %v", err) + } + deleter.UnlockOrDie() + + loader := StateFile{RootDir: dir, ID: found} + if err := loader.load(&Container{}, LoadOpts{}); !os.IsNotExist(err) { + t.Fatalf("load: got %v, want not-exist error", err) + } + if _, err := os.Stat(loader.lockPath()); !os.IsNotExist(err) { + t.Errorf("lock file %q still exists after loading a destroyed container", loader.lockPath()) + } +} + +// TestLoadExactMissing loads an ID that has no state file without globbing. +func TestLoadExactMissing(t *testing.T) { + dir := t.TempDir() + id := FullID{SandboxID: "sandbox", ContainerID: "container"} + + if _, err := Load(dir, id, LoadOpts{Exact: true}); !os.IsNotExist(err) { + t.Fatalf("Load: got %v, want not-exist error", err) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + if len(entries) != 0 { + t.Errorf("Load of a missing container left %d files in %q", len(entries), dir) + } +}