junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
63.4k stars 2.36k forks source link

panic: failed to find terminating 0 byte in dirent #3706

Closed 2084x closed 4 months ago

2084x commented 4 months ago

Checklist

Output of fzf --version

0.48.1 (9ffe951f)

OS

Shell

Problem / Steps to reproduce

I have a directory for all my music files (3699 folders, 115324 files). when running fzf in this directory or any of it's parent directories, this error is printed:

$ cd /mnt/media/music
$ fzf
panic: failed to find terminating 0 byte in dirent

                                                  goroutine 26 [running]:
                                                                         github.com/charlievieth/fastwalk.direntNamlen(...)
                                                                                                                                github.com/charlievieth/fastwalk@v1.0.2/fastwalk_unix_linux.go:28
                                  github.com/charlievieth/fastwalk.parseDirEnt({0xc00002e64c?, 0x610?, 0xc00001d380?})
                                                                                                                        github.com/charlievieth/fastwalk@v1.0.2/fastwalk_unix.go:120 +0x207
                            github.com/charlievieth/fastwalk.readDir({0x5ae711642690, 0x1}, 0xc00002fe80)
                                                                                                                github.com/charlievieth/fastwalk@v1.0.2/fastwalk_unix.go:48 +0x217
                   github.com/charlievieth/fastwalk.(*walker).walk(0xc00011e000, {0x5ae711642690, 0x1}, {0x5ae711698c80, 0xc000124000}, 0x1?)
                                                                                                                                                github.com/charlievieth/fastwalk@v1.0.2/fastwalk.go:376 +0xbb
                                              github.com/charlievieth/fastwalk.(*walker).doWork(0xc00011e000, 0x0?)
                                                                                                                        github.com/charlievieth/fastwalk@v1.0.2/fastwalk.go:257 +0x15c
                       created by github.com/charlievieth/fastwalk.Walk in goroutine 6
                                                                                        github.com/charlievieth/fastwalk@v1.0.2/fastwalk.go:202 +0x3c7

this error does not occur when piping into fzf like so: find /mnt/media/music | fzf

this error does not occur when running fzf in any other directory in /mnt/media or on any of my other drives.

junegunn commented 4 months ago

@charlievieth Any idea?

junegunn commented 4 months ago

I looked at the upstream repo, and noticed they removed fastwalk.

https://github.com/golang/tools/commit/1762c80ff22626c02decf68a2807db29be163f57

Anyway, the code before the removal was slightly different from what we have in charlievieth/fastwalk, but the panic was still there.

https://github.com/golang/tools/blob/9cf559ce9f9d243580e8c4e54d4c1b3a907dd781/internal/fastwalk/fastwalk_dirent_namlen_linux.go#L25-L27

We would need to just ignore the entry instead of panicking.

junegunn commented 4 months ago

I've removed panic()s in my fork of fastpath so the errors are ignored. Can you check if the problem is resolved in the devel branch?

2084x commented 4 months ago

there is no more panic on the devel branch, although I can now see that there are about 15,000 files missing from the list when comparing the results with 0.46 (before the find command was replaced with fastwalk).

junegunn commented 4 months ago

Thanks, that's better than an outright crash, I think. Can we get a clue on why some entries are not properly read by comparing the two lists?

vim -d <(find /mnt/media/music | sort) <(fzf --walker-root /mnt/media/music --filter '' --no-sort | sort)
2084x commented 4 months ago

I can't see any obvious reason why some files are being left out.

Here is the separate output of those commands: find /mnt/media/music -type f | sort > find.txt find.txt

fzf --walker-root /mnt/media/music --filter '' --no-sort | sort > fzf-0.48-devel.txt fzf-0.48-devel.txt

charlievieth commented 4 months ago

I'm looking into this now and one thing that I noticed is that fastwalk's logic for parsing dirents has drifted from what the Go stdlib uses. Basically, instead of copying (memove) the buf into a syscall.Dirent the stdlib now uses offsets into the buffer itself (os/dir_unix.go).

My guess is that something here could be the cause of the bug and regardless this library should not panic on error (that was an oversight on my part).

I'm going to update the dirent parsing logic of fastwalk to match the logic currently used by Go to see if that fixes the problem and will post a link to my branch when done.

I'll also take a look at @junegunn's PR https://github.com/charlievieth/fastwalk/pull/10 (thank you again for making that).

charlievieth commented 4 months ago

Could you trying this with my cev/unix-dirent branch (WIP PR https://github.com/charlievieth/fastwalk/pull/11)? It updates fastwalk's Dirent parsing logic to match the stdlibs (go1.22).

junegunn commented 4 months ago

@charlievieth Thanks for the quick turnaround!

@2084x Can you try again with the updated devel branch?

2084x commented 4 months ago

updated devel branch has fixed it! I now see the exact same number of files.

charlievieth commented 4 months ago

@2084x and @junegunn thank you both for finding/reporting this bug and for helping to test the fixes. It's pretty late my time but I'll cut a new release early tomorrow.

charlievieth commented 4 months ago

Thank you both for your help and your patience. I just cut fastwalk/v1.0.3 which resolves this issue and a darwin bug that was surfaced while trying to repro this issue https://github.com/charlievieth/fastwalk/pull/13 (hence the delay).

FZF PR to update fastwalk: https://github.com/junegunn/fzf/pull/3709

junegunn commented 4 months ago

@charlievieth Thanks a lot! I'll release a new version with the fix later today.