Skip to content

Commit

Permalink
Add lock sanity checks to Save() methods
Browse files Browse the repository at this point in the history
I have experienced "layer not known" corruption triggered by concurrent
buildah/skopeo processes, and hopefully lock sanity checks will help to
prevent this kind of problem.

Signed-off-by: Zac Medico <[email protected]>
  • Loading branch information
zmedico committed Aug 25, 2018
1 parent 17c7d1f commit c7ba574
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 2 deletions.
7 changes: 7 additions & 0 deletions containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ func (r *containerStore) Load() error {
}

func (r *containerStore) Save() error {
if !r.Locked() {
return errors.New("container store is not locked")
}
rpath := r.containerspath()
if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil {
return err
Expand Down Expand Up @@ -560,3 +563,7 @@ func (r *containerStore) IsReadWrite() bool {
func (r *containerStore) TouchedSince(when time.Time) bool {
return r.lockfile.TouchedSince(when)
}

func (r *containerStore) Locked() bool {
return r.lockfile.Locked()
}
7 changes: 7 additions & 0 deletions images.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ func (r *imageStore) Save() error {
if !r.IsReadWrite() {
return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to modify the image store at %q", r.imagespath())
}
if !r.Locked() {
return errors.New("image store is not locked")
}
rpath := r.imagespath()
if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil {
return err
Expand Down Expand Up @@ -701,3 +704,7 @@ func (r *imageStore) IsReadWrite() bool {
func (r *imageStore) TouchedSince(when time.Time) bool {
return r.lockfile.TouchedSince(when)
}

func (r *imageStore) Locked() bool {
return r.lockfile.Locked()
}
7 changes: 7 additions & 0 deletions layers.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ func (r *layerStore) Save() error {
if !r.IsReadWrite() {
return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to modify the layer store at %q", r.layerspath())
}
if !r.Locked() {
return errors.New("layer store is not locked")
}
rpath := r.layerspath()
if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil {
return err
Expand Down Expand Up @@ -1181,3 +1184,7 @@ func (r *layerStore) IsReadWrite() bool {
func (r *layerStore) TouchedSince(when time.Time) bool {
return r.lockfile.TouchedSince(when)
}

func (r *layerStore) Locked() bool {
return r.lockfile.Locked()
}
3 changes: 3 additions & 0 deletions lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ type Locker interface {

// IsReadWrite() checks if the lock file is read-write
IsReadWrite() bool

// Locked() checks if lock is locked
Locked() bool
}

var (
Expand Down
12 changes: 10 additions & 2 deletions lockfile_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ func getLockFile(path string, ro bool) (Locker, error) {
}
unix.CloseOnExec(fd)
if ro {
return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_RDLCK}, nil
return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_RDLCK, locked: false}, nil
}
return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_WRLCK}, nil
return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_WRLCK, locked: false}, nil
}

type lockfile struct {
Expand All @@ -36,6 +36,7 @@ type lockfile struct {
fd uintptr
lw string
locktype int16
locked bool
}

// Lock locks the lock file
Expand All @@ -48,6 +49,7 @@ func (l *lockfile) Lock() {
Pid: int32(os.Getpid()),
}
l.mu.Lock()
l.locked = true
for unix.FcntlFlock(l.fd, unix.F_SETLKW, &lk) != nil {
time.Sleep(10 * time.Millisecond)
}
Expand All @@ -65,9 +67,15 @@ func (l *lockfile) Unlock() {
for unix.FcntlFlock(l.fd, unix.F_SETLKW, &lk) != nil {
time.Sleep(10 * time.Millisecond)
}
l.locked = false
l.mu.Unlock()
}

// Check if lock is locked
func (l *lockfile) Locked() bool {
return l.locked
}

// Touch updates the lock file with the UID of the user
func (l *lockfile) Touch() error {
l.lw = stringid.GenerateRandomID()
Expand Down

0 comments on commit c7ba574

Please sign in to comment.