fix: avoid data races when deleting in background

8 files changed, 94 insertions, 2 deletions

diff --git a/pkg/analyze/file.go b/pkg/analyze/file.go
index 522d0a8..401a822 100644
--- a/pkg/analyze/file.go
+++ b/pkg/analyze/file.go
@@ -3,6 +3,7 @@ package analyze
 import (
 	"os"
 	"path/filepath"
+	"sync"
 	"time"
 
 	"github.com/dundee/gdu/v5/pkg/fs"
@@ -112,6 +113,16 @@ func (f *File) GetFiles() fs.Files {
 	return fs.Files{}
 }
 
+// GetFilesLocked returns all files in directory
+func (f *File) GetFilesLocked() fs.Files {
+	return f.GetFiles()
+}
+
+// RLock panics on file
+func (f *File) RLock() func() {
+	panic("SetFiles should not be called on file")
+}
+
 // SetFiles panics on file
 func (f *File) SetFiles(files fs.Files) {
 	panic("SetFiles should not be called on file")
@@ -133,6 +144,7 @@ type Dir struct {
 	BasePath  string
 	Files     fs.Files
 	ItemCount int
+	m         sync.RWMutex
 }
 
 // AddFile add item fo files
@@ -145,6 +157,14 @@ func (f *Dir) GetFiles() fs.Files {
 	return f.Files
 }
 
+// GetFilesLocked returns all files in directory
+// It is safe to call this function from multiple goroutines
+func (f *Dir) GetFilesLocked() fs.Files {
+	f.m.RLock()
+	defer f.m.RUnlock()
+	return f.GetFiles()[:]
+}
+
 // SetFiles sets files in directory
 func (f *Dir) SetFiles(files fs.Files) {
 	f.Files = files
@@ -157,6 +177,8 @@ func (f *Dir) GetType() string {
 
 // GetItemCount returns number of files in dir
 func (f *Dir) GetItemCount() int {
+	f.m.RLock()
+	defer f.m.RUnlock()
 	return f.ItemCount
 }
 
@@ -211,6 +233,9 @@ func (f *Dir) UpdateStats(linkedItems fs.HardLinkedItems) {
 
 // RemoveFile panics on file
 func (f *Dir) RemoveFile(item fs.Item) {
+	f.m.Lock()
+	defer f.m.Unlock()
+
 	f.SetFiles(f.GetFiles().Remove(item))
 
 	cur := f
@@ -226,6 +251,12 @@ func (f *Dir) RemoveFile(item fs.Item) {
 	}
 }
 
+// RLock read locks dir
+func (f *Dir) RLock() func() {
+	f.m.RLock()
+	return f.m.RUnlock
+}
+
 // RemoveItemFromDir removes item from dir
 func RemoveItemFromDir(dir fs.Item, item fs.Item) error {
 	err := os.RemoveAll(item.GetPath())
diff --git a/pkg/analyze/file_test.go b/pkg/analyze/file_test.go
index 57d87d1..e5652c4 100644
--- a/pkg/analyze/file_test.go
+++ b/pkg/analyze/file_test.go
@@ -412,6 +412,30 @@ func TestGetFiles(t *testing.T) {
 	assert.Equal(t, fs.Files{}, file.GetFiles())
 }
 
+func TestGetFilesLocked(t *testing.T) {
+	file := &File{
+		Name: "xxx",
+		Mli:  5,
+	}
+	dir := &Dir{
+		File: &File{
+			Name:  "root",
+			Size:  5,
+			Usage: 12,
+		},
+		ItemCount: 3,
+		BasePath:  "/",
+		Files:     fs.Files{file},
+	}
+
+	unlock := dir.RLock()
+	defer unlock()
+	files := dir.GetFiles()
+	locked := dir.GetFilesLocked()
+	files = files.Remove(file)
+	assert.NotEqual(t, &files, &locked)
+}
+
 func TestSetFilesPanicsOnFile(t *testing.T) {
 	file := &File{
 		Name: "xxx",
diff --git a/pkg/analyze/stored.go b/pkg/analyze/stored.go
index 14182c5..70dba76 100644
--- a/pkg/analyze/stored.go
+++ b/pkg/analyze/stored.go
@@ -373,6 +373,8 @@ func (p *ParentDir) EncodeJSON(writer io.Writer, topLevel bool) error { panic("m
 func (p *ParentDir) UpdateStats(linkedItems fs.HardLinkedItems)       { panic("must not be called") }
 func (p *ParentDir) AddFile(fs.Item)                                  { panic("must not be called") }
 func (p *ParentDir) GetFiles() fs.Files                               { panic("must not be called") }
+func (p *ParentDir) GetFilesLocked() fs.Files                         { panic("must not be called") }
+func (p *ParentDir) RLock() func()                                    { panic("must not be called") }
 func (p *ParentDir) SetFiles(fs.Files)                                { panic("must not be called") }
 func (f *ParentDir) RemoveFile(item fs.Item)                          { panic("must not be called") }
 func (p *ParentDir) GetItemStats(
diff --git a/pkg/fs/file.go b/pkg/fs/file.go
index 368ddaa..7d286b0 100644
--- a/pkg/fs/file.go
+++ b/pkg/fs/file.go
@@ -26,8 +26,10 @@ type Item interface {
 	UpdateStats(linkedItems HardLinkedItems)
 	AddFile(Item)
 	GetFiles() Files
+	GetFilesLocked() Files
 	SetFiles(Files)
 	RemoveFile(Item)
+	RLock() func()
 }
 
 // Files - slice of pointers to File
diff --git a/tui/background.go b/tui/background.go
index 813c3b9..c62d005 100644
--- a/tui/background.go
+++ b/tui/background.go
@@ -44,7 +44,7 @@ func (ui *UI) deleteItem(item fs.Item, shouldEmpty bool) {
 	var deleteItems []fs.Item
 	if shouldEmpty && item.IsDir() {
 		parentDir = item.(*analyze.Dir)
-		for _, file := range item.GetFiles() {
+		for _, file := range item.GetFilesLocked() {
 			deleteItems = append(deleteItems, file)
 		}
 	} else {
diff --git a/tui/show.go b/tui/show.go
index ce14e16..39923a1 100644
--- a/tui/show.go
+++ b/tui/show.go
@@ -84,6 +84,9 @@ func (ui *UI) showDir() {
 
 	ui.sortItems()
 
+	unlock := ui.currentDir.RLock()
+	defer unlock()
+
 	if ui.ShowRelativeSize {
 		for _, item := range ui.currentDir.GetFiles() {
 			if item.GetUsage() > maxUsage {
diff --git a/tui/tui.go b/tui/tui.go
index a0facec..b7a376a 100644
--- a/tui/tui.go
+++ b/tui/tui.go
@@ -68,6 +68,7 @@ type UI struct {
 	activeWorkers           int
 	workersMut              sync.Mutex
 	statusMut               sync.RWMutex
+	deleteWorkersCount      int
 }
 
 type deleteQueueItem struct {
@@ -117,6 +118,7 @@ func CreateUI(
 		exportName:              "export.json",
 		noDelete:                false,
 		deleteQueue:             make(chan deleteQueueItem, 1000),
+		deleteWorkersCount:      3 * runtime.GOMAXPROCS(0),
 	}
 	for _, o := range opts {
 		o(ui)
@@ -233,7 +235,7 @@ func (ui *UI) SetNoDelete() {
 func (ui *UI) SetDeleteInBackground() {
 	ui.deleteInBackground = true
 
-	for i := 0; i < 3*runtime.GOMAXPROCS(0); i++ {
+	for i := 0; i < ui.deleteWorkersCount; i++ {
 		go ui.deleteWorker()
 	}
 	go ui.updateStatusWorker()
diff --git a/tui/tui_test.go b/tui/tui_test.go
index ecd0c8f..d02c949 100644
--- a/tui/tui_test.go
+++ b/tui/tui_test.go
@@ -511,6 +511,34 @@ func TestDeleteMarkedWithErr(t *testing.T) {
 	assert.DirExists(t, "test_dir/nested")
 }
 
+func TestDeleteMarkedInBackground(t *testing.T) {
+	fin := testdir.CreateTestDir()
+	defer fin()
+
+	ui := getAnalyzedPathMockedApp(t, false, true, false)
+	ui.SetDeleteInBackground()
+
+	assert.Equal(t, 1, ui.table.GetRowCount())
+
+	ui.fileItemSelected(0, 0) // nested
+
+	ui.markedRows[1] = struct{}{} // subnested
+	ui.markedRows[2] = struct{}{} // file2
+
+	ui.deleteMarked(false)
+
+	<-ui.done // wait for deletion of subnested
+	<-ui.done // wait for deletion of file2
+
+	for _, f := range ui.app.(*testapp.MockedApp).GetUpdateDraws() {
+		f()
+	}
+
+	assert.DirExists(t, "test_dir/nested")
+	assert.NoDirExists(t, "test_dir/nested/subnested")
+	assert.NoFileExists(t, "test_dir/nested/file2")
+}
+
 func TestDeleteMarkedInBackgroundWithErr(t *testing.T) {
 	fin := testdir.CreateTestDir()
 	defer fin()