fsnotify 1.8.0 crashes lf on symbolic links

[valoq]

When using the new fsnotify version from the current master branch and coming across a path that contains a symbolic link while the watch option is set to true, lf crashes with no useful information in the logfile. I suspect this may actually be a bug in fsnotify but I was not able to trigger any crash when using the example fsnotfy code.

Fortunately the issue does not affect the lf release version v33

[joelim-work]

I was able to reproduce a crash too, by changing to / where there are symbolic links, and then changing to another directory. The crash happens when removing watches, since lf stops watching directories when they are no longer 'visible':

https://github.com/gokcehan/lf/blob/dda8e88aafd4ff5370dbfac1b61fb193c329cbd0/watch.go#L72-L76

When saving the stack trace, I found that the panic occurs inside fsnotify when calling Watcher.Remove:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x5aac8d]

goroutine 1 [running]:
github.com/fsnotify/fsnotify.(*watches).removePath(0xc0000abd70, {0xc0005c8116, 0x5})
    /home/joelim/go/pkg/mod/github.com/fsnotify/fsnotify@v1.8.0/backend_inotify.go:119 +0x22d
github.com/fsnotify/fsnotify.(*inotify).remove(0xc0000b6000, {0xc0005c8116?, 0xc0000abd40?})
    /home/joelim/go/pkg/mod/github.com/fsnotify/fsnotify@v1.8.0/backend_inotify.go:372 +0x28
github.com/fsnotify/fsnotify.(*inotify).Remove(0xc0000b6000, {0xc0005c8116, 0x5})
    /home/joelim/go/pkg/mod/github.com/fsnotify/fsnotify@v1.8.0/backend_inotify.go:368 +0xea
github.com/fsnotify/fsnotify.(*Watcher).Remove(...)
    /home/joelim/go/pkg/mod/github.com/fsnotify/fsnotify@v1.8.0/fsnotify.go:332
main.(*watch).set(0xc000098050, 0xc00078ec10)
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/watch.go:74 +0xbb
main.(*app).setWatchPaths(0xc0002d8000)
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/app.go:648 +0x1b6
main.onChdir(0xc0002d8000)
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/eval.go:584 +0x29
main.(*callExpr).eval(0xc0000aa9f0, 0xc0002d8000, {0xc00090e000?, 0x0?, 0xc0000bda00?})
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/eval.go:1099 +0xc13
main.(*app).loop(0xc0002d8000)
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/app.go:471 +0xf73
main.run()
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/client.go:65 +0x30f
main.main()
    /home/joelim/go/pkg/mod/github.com/gokcehan/lf@v0.0.0-20241101132854-dda8e88aafd4/main.go:357 +0x819

I was also able to get fsnotify to crash with a test program:

package main

import (
    "log"

    "github.com/fsnotify/fsnotify"
)

func main() {
    watcher, err := fsnotify.NewWatcher()
    if err != nil {
        log.Fatal(err)
    }
    defer watcher.Close()

    // both /lib and /lib64 are symlinked to /usr/lib on my machine
    watcher.Add("/lib")
    watcher.Add("/lib64")

    // crash occurs only when both of the following lines are run
    watcher.Remove("/lib")
    watcher.Remove("/lib64")
}

Can you try checking if fsnotify causes similar issues for you as well? Might have to raise an issue with them. Otherwise I don't think there's much I can do specifically for lf - the best I can do is downgrade fsnotify back to version 1.7.0 for now.

[valoq]

I could reproduce the same stack trace with the example code as well and opened an issue at https://github.com/fsnotify/fsnotify/issues/652

[joelim-work]

I created a workaround #1843, not sure if it completely works. It is really only temporary though, I would prefer to have the upstream fsnotify library not panic at all.

[valoq]

Since this appears to be a undisputed bug in the upstream fsnotify implementation, I do not think we need a workaround. However we may want to revert back to fsnotify version 1.7.0 in the master branch to avoid the problem until a new fixed version is released.

[joelim-work]

I merged your PR for now, but pinning the dependency like this is only good as a short-term solution. Unless that bug is fixed, or a workaround is added in lf to suppress the panic, then it's not possible to use a newer version of fsnotify, even if it contains something important like a patch to fix a security vulnerability.