Merge pull request #4985 from butonic/drop-unneeded-locks

drop unneeded session locks
cs3org · Dec 11, 2024 · 5a68fdf · 5a68fdf
2 parents 2c0b4ea + 8c1b7f6
commit 5a68fdf
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 23 deletions.
diff --git a/changelog/unreleased/drop-unneeded-locks.md b/changelog/unreleased/drop-unneeded-locks.md
@@ -0,0 +1,5 @@
+Bugfix: drop unneeded session locks
+
+We no longer lock session metadada files, as they are already written atomically.
+
+https://github.com/cs3org/reva/pull/4985
diff --git a/pkg/storage/utils/decomposedfs/upload/session.go b/pkg/storage/utils/decomposedfs/upload/session.go
@@ -27,7 +27,6 @@ import (
 	"time"
 
 	"github.com/google/renameio/v2"
-	"github.com/rogpeppe/go-internal/lockedfile"
 	tusd "github.com/tus/tusd/v2/pkg/handler"
 
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -78,14 +77,6 @@ func (s *OcisSession) Purge(ctx context.Context) error {
 	_, span := tracer.Start(ctx, "Purge")
 	defer span.End()
 	sessionPath := sessionPath(s.store.root, s.info.ID)
-	f, err := lockedfile.OpenFile(sessionPath+".lock", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0600)
-	if err != nil {
-		return err
-	}
-	defer func() {
-		f.Close()
-		os.Remove(sessionPath + ".lock")
-	}()
 	if err := os.Remove(sessionPath); err != nil {
 		return err
 	}
@@ -121,11 +112,6 @@ func (s *OcisSession) Persist(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	f, err := lockedfile.OpenFile(sessionPath+".lock", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0600)
-	if err != nil {
-		return err
-	}
-	defer f.Close()
 	return renameio.WriteFile(sessionPath, d, 0600)
 }
 

diff --git a/pkg/storage/utils/decomposedfs/upload/store.go b/pkg/storage/utils/decomposedfs/upload/store.go
@@ -28,6 +28,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
@@ -132,24 +133,19 @@ func (store OcisStore) Get(ctx context.Context, id string) (*OcisSession, error)
 		store: store,
 		info:  tusd.FileInfo{},
 	}
-	lock, err := lockedfile.Open(sessionPath + ".lock")
+	data, err := os.ReadFile(sessionPath)
 	if err != nil {
-		if errors.Is(err, iofs.ErrNotExist) {
-			// Interpret os.ErrNotExist as 404 Not Found
+		// handle stale NFS file handles that can occur when the file is deleted betwenn the ATTR and FOPEN call of os.ReadFile
+		if pathErr, ok := err.(*os.PathError); ok && pathErr.Err == syscall.ESTALE {
+			appctx.GetLogger(ctx).Info().Str("session", id).Err(err).Msg("treating stale file handle as not found")
 			err = tusd.ErrNotFound
 		}
-		return nil, err
-	}
-	defer lock.Close()
-	data, err := os.ReadFile(sessionPath)
-	if err != nil {
 		if errors.Is(err, iofs.ErrNotExist) {
 			// Interpret os.ErrNotExist as 404 Not Found
 			err = tusd.ErrNotFound
 		}
 		return nil, err
 	}
-	lock.Close() // release lock asap
 
 	if err := json.Unmarshal(data, &session.info); err != nil {
 		return nil, err