fix(fs): fd must be closed after deleting, renaming

src/fs/local/fs.go

@@ -15,7 +15,7 @@ import (
 	"github.com/ihexxa/quickshare/src/idgen"
 )
 
-var ErrTooManyOpens = errors.New("too many opened files")
+var ErrTooManyOpens = errors.New("too many opened files and failed to clean")
 
 type LocalFS struct {
 	root           string
@@ -148,6 +148,35 @@ func (fs *LocalFS) Sync() error {
 	return nil
 }
 
+func (fs *LocalFS) CloseFDs(filePaths map[string]bool) error {
+	fs.opensMtx.Lock()
+	defer fs.opensMtx.Unlock()
+	return fs.closeFDs(filePaths)
+}
+
+func (fs *LocalFS) closeFDs(filePaths map[string]bool) error {
+	var err error
+	for filePath := range filePaths {
+		info, ok := fs.opens[filePath]
+		if ok {
+			err = fs.closeInfo(filePath, info)
+			if err != nil {
+				return err
+			}
+		}
+
+		info, ok = fs.readers[filePath]
+		if ok {
+			err = fs.closeInfo(filePath, info)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func (fs *LocalFS) Close() error {
 	fs.opensMtx.Lock()
 	defer fs.opensMtx.Unlock()
@@ -171,7 +200,7 @@ func (fs *LocalFS) Create(path string) error {
 	if fs.isTooManyOpens() {
 		closed, err := fs.closeOpens(false, false, map[string]bool{})
 		if err != nil || closed == 0 {
-			return fmt.Errorf("too many opens and fail to clean(%d): %w", closed, err)
+			return ErrTooManyOpens
 		}
 	}
 
@@ -209,7 +238,12 @@ func (fs *LocalFS) Remove(entryPath string) error {
 	if err != nil {
 		return err
 	}
-	return os.RemoveAll(fullpath)
+
+	err = os.RemoveAll(fullpath)
+	if err != nil {
+		return err
+	}
+	return fs.CloseFDs(map[string]bool{fullpath: true})
 }
 
 func (fs *LocalFS) Rename(oldpath, newpath string) error {
@@ -231,7 +265,14 @@ func (fs *LocalFS) Rename(oldpath, newpath string) error {
 	_, err = os.Stat(fullNewPath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			return os.Rename(fullOldPath, fullNewPath)
+			err = os.Rename(fullOldPath, fullNewPath)
+			if err != nil {
+				return err
+			}
+			err = fs.CloseFDs(map[string]bool{fullOldPath: true})
+			if err != nil {
+				return err
+			}
 		}
 		return err
 	}
@@ -253,7 +294,7 @@ func (fs *LocalFS) ReadAt(path string, b []byte, off int64) (int, error) {
 			if fs.isTooManyOpens() {
 				closed, err := fs.closeOpens(false, false, map[string]bool{})
 				if err != nil || closed == 0 {
-					return nil, fmt.Errorf("too many opens and fail to clean (%d): %w", closed, err)
+					return nil, ErrTooManyOpens
 				}
 			}
 
@@ -304,7 +345,7 @@ func (fs *LocalFS) WriteAt(path string, b []byte, off int64) (int, error) {
 				closed, err := fs.closeOpens(false, false, map[string]bool{})
 				if err != nil || closed == 0 {
 					// TODO: return Eagain and make client retry later
-					return nil, fmt.Errorf("too many opens and fail to clean (%d): %w", closed, err)
+					return nil, ErrTooManyOpens
 				}
 			}
 
@@ -367,7 +408,7 @@ func (fs *LocalFS) GetFileReader(path string) (fs.ReadCloseSeeker, uint64, error
 	if fs.isTooManyOpens() {
 		closed, err := fs.closeOpens(false, false, map[string]bool{})
 		if err != nil || closed == 0 {
-			return nil, 0, fmt.Errorf("too many opens and fail to clean (%d): %w", closed, err)
+			return nil, 0, ErrTooManyOpens
 		}
 	}
 

src/handlers/fileshdr/handlers.go

@@ -157,6 +157,7 @@ func (h *FileHandlers) Create(c *gin.Context) {
 
 		err = h.deps.FS().Create(tmpFilePath)
 		if err != nil {
+			fmt.Println("\nexist,", err, tmpFilePath)
 			if os.IsExist(err) {
 				// avoid adding file size more than once
 				err = h.deps.Users().SetUsed(userIDInt, false, req.FileSize)

src/server/server_files_test.go

@@ -155,6 +155,39 @@ func TestFileHandlers(t *testing.T) {
 		}
 	})
 
+	t.Run("test: Upload-Delete-Upload: fd are closed correctly", func(t *testing.T) {
+		files := map[string]string{
+			"qs/files/close_Fd/dup_file1": "12345678",
+		}
+
+		for i := 0; i < 2; i++ {
+			for filePath, content := range files {
+				assertUploadOK(t, filePath, content, addr, token)
+
+				// file operation(deleting tmp file) may be async
+				// so creating tmp file in the 2nd time may conflict with the first time if it is not deleted yet
+				// checking file until it is deleted
+				// TODO: use fs.Stat() to avoid flaky testing...
+				time.Sleep(1000)
+
+				err = fs.Close()
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				res, _, errs := cl.Delete(filePath)
+				if len(errs) > 0 {
+					t.Fatal(errs)
+				} else if res.StatusCode != 200 {
+					t.Fatal(res.StatusCode)
+				}
+
+				// tmpFile fd is closed, so follow must succeed
+				assertUploadOK(t, filePath, content, addr, token)
+			}
+		}
+	})
+
 	t.Run("test files APIs: Create-UploadChunk-UploadStatus-Metadata-Delete", func(t *testing.T) {
 		for filePath, content := range map[string]string{
 			"qs/files/path1/f1.md":       "1111 1111 1111 1111",