Skip to content

Commit

Permalink
Fix #4260: Synchronize access to fileDialog.data file list (#4279)
Browse files Browse the repository at this point in the history
This change adds a RWMutex to synchronize access to the fileDialog.data file list.
The refreshDir function clears and rebuilds fileDialog.data, meanwhile, several other functions are indexing into it.
This allowed for scenarios where fileDialog.data was either nil or smaller than expected when indexing into it.

Fixes #4260
  • Loading branch information
JordanGoulder authored Sep 28, 2023
1 parent 767105e commit f17ccd8
Showing 1 changed file with 32 additions and 7 deletions.
39 changes: 32 additions & 7 deletions dialog/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"runtime"
"strings"
"sync"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/container"
Expand Down Expand Up @@ -56,7 +57,9 @@ type fileDialog struct {
showHidden bool

view viewLayout
data []fyne.URI

data []fyne.URI
dataLock sync.RWMutex

win *widget.PopUp
selected fyne.URI
Expand Down Expand Up @@ -365,7 +368,9 @@ func (f *fileDialog) loadFavorites() {
}

func (f *fileDialog) refreshDir(dir fyne.ListableURI) {
f.dataLock.Lock()
f.data = nil
f.dataLock.Unlock()

files, err := dir.List()
if err != nil {
Expand Down Expand Up @@ -397,7 +402,10 @@ func (f *fileDialog) refreshDir(dir fyne.ListableURI) {
icons = append(icons, file)
}
}

f.dataLock.Lock()
f.data = icons
f.dataLock.Unlock()

f.files.Refresh()
f.filesScroll.Offset = fyne.NewPos(0, 0)
Expand Down Expand Up @@ -513,20 +521,26 @@ func (f *fileDialog) setView(view viewLayout) {
fyne.CurrentApp().Preferences().SetInt(viewLayoutKey, int(view))

count := func() int {
f.dataLock.RLock()
defer f.dataLock.RUnlock()

return len(f.data)
}
template := func() fyne.CanvasObject {
return f.newFileItem(storage.NewFileURI("./tempfile"), true, false)
}
update := func(id widget.GridWrapItemID, o fyne.CanvasObject) {
dir := f.data[id]
parent := id == 0 && len(dir.Path()) < len(f.dir.Path())
_, isDir := dir.(fyne.ListableURI)
o.(*fileDialogItem).setLocation(dir, isDir || parent, parent)
if dir, ok := f.getDataItem(id); ok {
parent := id == 0 && len(dir.Path()) < len(f.dir.Path())
_, isDir := dir.(fyne.ListableURI)
o.(*fileDialogItem).setLocation(dir, isDir || parent, parent)
}
}
choose := func(id int) {
f.selectedID = id
f.setSelected(f.data[id], id)
if file, ok := f.getDataItem(id); ok {
f.selectedID = id
f.setSelected(file, id)
}
}
if f.view == gridView {
grid := widget.NewGridWrap(count, template, update)
Expand All @@ -544,6 +558,17 @@ func (f *fileDialog) setView(view viewLayout) {
f.filesScroll.Refresh()
}

func (f *fileDialog) getDataItem(id int) (fyne.URI, bool) {
f.dataLock.RLock()
defer f.dataLock.RUnlock()

if id >= len(f.data) {
return nil, false
}

return f.data[id], true
}

// effectiveStartingDir calculates the directory at which the file dialog should
// open, based on the values of startingDirectory, CWD, home, and any error
// conditions which occur.
Expand Down

0 comments on commit f17ccd8

Please sign in to comment.