From 1a5af8cafd4d465d5cf670f6ffaaaecc08b4b6c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Magiera?= <magik6k@gmail.com>
Date: Mon, 8 Jun 2020 18:47:59 +0200
Subject: [PATCH] Improve fault checker

---
 faults.go             | 33 +++++++++++++++++++++++++++++--
 stores/index.go       |  1 +
 stores/index_locks.go | 46 +++++++++++++++++++++++++++++++++++--------
 3 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/faults.go b/faults.go
index 6bf036089..635ffa0c3 100644
--- a/faults.go
+++ b/faults.go
@@ -2,6 +2,8 @@ package sectorstorage
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 
 	"golang.org/x/xerrors"
 
@@ -19,9 +21,22 @@ func (m *Manager) CheckProvable(ctx context.Context, spt abi.RegisteredProof, se
 	var bad []abi.SectorID
 
 	// TODO: More better checks
-	// TODO: Use proper locking
 	for _, sector := range sectors {
 		err := func() error {
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
+
+			locked, err := m.index.StorageTryLock(ctx, sector, stores.FTSealed|stores.FTCache, stores.FTNone)
+			if err != nil {
+				return xerrors.Errorf("acquiring sector lock: %w", err)
+			}
+
+			if !locked {
+				log.Warnw("CheckProvable Sector FAULT: can't acquire read lock", "sector", sector, "sealed")
+				bad = append(bad, sector)
+				return nil
+			}
+
 			lp, _, err := m.localStore.AcquireSector(ctx, sector, spt, stores.FTSealed|stores.FTCache, stores.FTNone, false, stores.AcquireMove)
 			if err != nil {
 				return xerrors.Errorf("acquire sector in checkProvable: %w", err)
@@ -33,7 +48,21 @@ func (m *Manager) CheckProvable(ctx context.Context, spt abi.RegisteredProof, se
 				return nil
 			}
 
-			// must be fine
+			toCheck := []string{
+				lp.Sealed,
+				filepath.Join(lp.Cache, "t_aux"),
+				filepath.Join(lp.Cache, "p_aux"),
+				filepath.Join(lp.Cache, "sc-02-data-tree-r-last.dat"),
+			}
+
+			for _, p := range toCheck {
+				_, err := os.Stat(p)
+				if err != nil {
+					log.Warnw("CheckProvable Sector FAULT: sector file stat error", "sector", sector, "sealed", lp.Sealed, "cache", lp.Cache, "file", p)
+					bad = append(bad, sector)
+					return nil
+				}
+			}
 
 			return nil
 		}()
diff --git a/stores/index.go b/stores/index.go
index 6ef346367..e113a005a 100644
--- a/stores/index.go
+++ b/stores/index.go
@@ -62,6 +62,7 @@ type SectorIndex interface { // part of storage-miner api
 
 	// atomically acquire locks on all sector file types. close ctx to unlock
 	StorageLock(ctx context.Context, sector abi.SectorID, read SectorFileType, write SectorFileType) error
+	StorageTryLock(ctx context.Context, sector abi.SectorID, read SectorFileType, write SectorFileType) (bool, error)
 }
 
 type Decl struct {
diff --git a/stores/index_locks.go b/stores/index_locks.go
index 8e1a07a02..8bf15b950 100644
--- a/stores/index_locks.go
+++ b/stores/index_locks.go
@@ -45,17 +45,26 @@ func (l *sectorLock) tryLock(read SectorFileType, write SectorFileType) bool {
 	return true
 }
 
-func (l *sectorLock) lock(ctx context.Context, read SectorFileType, write SectorFileType) error {
+type lockFn func(l *sectorLock, ctx context.Context, read SectorFileType, write SectorFileType) (bool, error)
+
+func (l *sectorLock) tryLockSafe(ctx context.Context, read SectorFileType, write SectorFileType) (bool, error) {
+	l.cond.L.Lock()
+	defer l.cond.L.Unlock()
+
+	return l.tryLock(read, write), nil
+}
+
+func (l *sectorLock) lock(ctx context.Context, read SectorFileType, write SectorFileType) (bool, error) {
 	l.cond.L.Lock()
 	defer l.cond.L.Unlock()
 
 	for !l.tryLock(read, write) {
 		if err := l.cond.Wait(ctx); err != nil {
-			return err
+			return false, err
 		}
 	}
 
-	return nil
+	return true, nil
 }
 
 func (l *sectorLock) unlock(read SectorFileType, write SectorFileType) {
@@ -79,13 +88,13 @@ type indexLocks struct {
 	locks map[abi.SectorID]*sectorLock
 }
 
-func (i *indexLocks) StorageLock(ctx context.Context, sector abi.SectorID, read SectorFileType, write SectorFileType) error {
+func (i *indexLocks) lockWith(ctx context.Context, lockFn lockFn, sector abi.SectorID, read SectorFileType, write SectorFileType) (bool, error) {
 	if read|write == 0 {
-		return nil
+		return false, nil
 	}
 
 	if read|write > (1<<FileTypes)-1 {
-		return xerrors.Errorf("unknown file types specified")
+		return false, xerrors.Errorf("unknown file types specified")
 	}
 
 	i.lk.Lock()
@@ -100,8 +109,12 @@ func (i *indexLocks) StorageLock(ctx context.Context, sector abi.SectorID, read
 
 	i.lk.Unlock()
 
-	if err := slk.lock(ctx, read, write); err != nil {
-		return err
+	locked, err := lockFn(slk, ctx, read, write)
+	if err != nil {
+		return false, err
+	}
+	if !locked {
+		return false, nil
 	}
 
 	go func() {
@@ -120,5 +133,22 @@ func (i *indexLocks) StorageLock(ctx context.Context, sector abi.SectorID, read
 		i.lk.Unlock()
 	}()
 
+	return true, nil
+}
+
+func (i *indexLocks) StorageLock(ctx context.Context, sector abi.SectorID, read SectorFileType, write SectorFileType) error {
+	ok, err := i.lockWith(ctx, (*sectorLock).lock, sector, read, write)
+	if err != nil {
+		return err
+	}
+
+	if !ok {
+		return xerrors.Errorf("failed to acquire lock")
+	}
+
 	return nil
 }
+
+func (i *indexLocks) StorageTryLock(ctx context.Context, sector abi.SectorID, read SectorFileType, write SectorFileType) (bool, error) {
+	return i.lockWith(ctx, (*sectorLock).tryLockSafe, sector, read, write)
+}