drop unneeded session locks

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
butonic · Dec 3, 2024 · 8c1b7f6 · 8c1b7f6
1 parent 17f3395
commit 8c1b7f6
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"
@@ -84,14 +83,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
 	}
@@ -127,11 +118,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"
@@ -129,24 +130,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