Skip to content

Commit

Permalink
Fix infinite loop when the fs fails. Closes #1164
Browse files Browse the repository at this point in the history
  • Loading branch information
deluan committed Jul 16, 2021
1 parent b0fc684 commit eb8ffc6
Showing 1 changed file with 16 additions and 20 deletions.
36 changes: 16 additions & 20 deletions scanner/walk_dir_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,6 @@ type (
walkResults = chan dirStats
)

func fullReadDir(name string) ([]os.DirEntry, error) {
f, err := os.Open(name)
if err != nil {
return nil, err
}
defer f.Close()

var allDirs []os.DirEntry
for {
dirs, err := f.ReadDir(-1)
allDirs = append(allDirs, dirs...)
if err == nil {
break
}
log.Warn("Skipping DirEntry", err)
}
sort.Slice(allDirs, func(i, j int) bool { return allDirs[i].Name() < allDirs[j].Name() })
return allDirs, nil
}

func walkDirTree(ctx context.Context, rootFolder string, results walkResults) error {
err := walkFolder(ctx, rootFolder, rootFolder, results)
if err != nil {
Expand Down Expand Up @@ -120,6 +100,22 @@ func loadDir(ctx context.Context, dirPath string) ([]string, *dirStats, error) {
return children, stats, nil
}

func fullReadDir(name string) ([]os.DirEntry, error) {
f, err := os.Open(name)
if err != nil {
return nil, err
}
defer f.Close()

dirs, err := f.ReadDir(-1)
if err != nil {
log.Warn("Skipping DirEntry", err)
return nil, nil
}
sort.Slice(dirs, func(i, j int) bool { return dirs[i].Name() < dirs[j].Name() })
return dirs, nil
}

// isDirOrSymlinkToDir returns true if and only if the dirEnt represents a file
// system directory, or a symbolic link to a directory. Note that if the dirEnt
// is not a directory but is a symbolic link, this method will resolve by
Expand Down

1 comment on commit eb8ffc6

@whorfin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deluan This will break the "find all subdirs when some are missing rx permissions" behavior fixed by #1054
The loop over ReadDir was not a bug.
https://pkg.go.dev/os#File.ReadDir

ReadDir reads the contents of the directory associated with the file f and returns a slice of DirEntry values in directory order. Subsequent calls on the same file will yield later DirEntry records in the directory.
...
If n <= 0, ReadDir returns all the DirEntry records remaining in the directory. When it succeeds, it returns a nil error (not io.EOF).

#1164 reports a scenario where mounts appear to be broken; the golang behavior here doesn't seem right (it should "succeed" and not remain stuck on the same directoryentry), but the fix is not to bail on the first error - there may be more entries available with a properly functioning filesystem.

The guts of this bug appear in this case limited to the unix os, so it looks like, again just for this case, it might also work to return when
the error is a PathError and the Op is "readdirent"
An alternative hack/workaround/sanity-check would be to look for identical, successive errors (full error string, including the failing path), and bail, assuming that to have been an impossible occurance

Please sign in to comment.