radovskyb / watcher

watcher is a Go package for watching for files or directory changes without using filesystem events.
BSD 3-Clause "New" or "Revised" License
1.48k stars 183 forks source link

fatal error: sync: unlock of unlocked mutex #121

Open qknight opened 1 year ago

qknight commented 1 year ago

Describe the bug A clear and concise description of what the bug is.

I changed a file which was observed by fsNotify and then the error occured:

fatal error: sync: unlock of unlocked mutex

goroutine 387 [running]:
sync.fatal({0xcb9384?, 0xce8238?})
        C:/Users/qknight/sdk/go1.19.3/src/runtime/panic.go:1031 +0x1e
sync.(*Mutex).unlockSlow(0xc0004680b8, 0xffffffff)
        C:/Users/qknight/sdk/go1.19.3/src/sync/mutex.go:229 +0x3c
sync.(*Mutex).Unlock(0x0?)
        C:/Users/qknight/sdk/go1.19.3/src/sync/mutex.go:223 +0x29
panic({0xc45760, 0xc000494690})
        C:/Users/qknight/sdk/go1.19.3/src/runtime/panic.go:884 +0x212
github.com/radovskyb/watcher.(*Watcher).retrieveFileList(0xc000168200)
        C:/Users/qknight/go/pkg/mod/github.com/radovskyb/watcher@v1.0.7/watcher.go:514 +0x56b
github.com/radovskyb/watcher.(*Watcher).Start(0xc000168200, 0x5f5e100)
        C:/Users/qknight/go/pkg/mod/github.com/radovskyb/watcher@v1.0.7/watcher.go:563 +0x158
main.fsNotifyWatchDocumentsDirectory({0xc0000285f0, 0x42})
        C:/Users/qknight/Desktop/Projects/pankat/cmd/pankat-server/fsNotify.go:63 +0xee
created by main.main
        C:/Users/qknight/Desktop/Projects/pankat/cmd/pankat-server/main.go:33 +0x31a

https://github.com/nixcloud/pankat/blob/future/cmd/pankat-server/fsNotify.go#L63

To Reproduce

git clone https://github.com/nixcloud/pankat.git
git checkout future
build pankat (using goland)
pankat-server --documents documents/blog.lastlog.de/

Expected behavior No crash, fsNotify just pointing out what files changed. This works very often, but sometimes crashes.

Which operating system and version are you using?

OS Name:                   Microsoft Windows 10 Pro
OS Version:                10.0.19044 N/A Build 19044
OS Manufacturer:           Microsoft Corporation
OS Configuration:          Standalone Workstation
OS Build Type:             Multiprocessor Free

https://github.com/radovskyb/watcher#example

qknight commented 1 year ago

Found a solution.

Triggered the bug with this

cp foo.mdwn foo1.mdwn
rm foo1.mdwn
cp foo.mdwn foo1.mdwn
rm foo1.mdwn
cp foo.mdwn foo1.mdwn
rm foo1.mdwn
cp foo.mdwn foo1.mdwn
rm foo1.mdwn
cp foo.mdwn foo1.mdwn
rm foo1.mdwn
cp foo.mdwn foo1.mdwn
rm foo1.mdwn

I found another occation created a slightly different error message which helped to identify the real issue:

panic: interface conversion: error is syscall.Errno, not *fs.PathError

goroutine 1337 [running]:
github.com/radovskyb/watcher.(*Watcher).retrieveFileList(0xc00011e080)
        C:/Users/qknight/go/pkg/mod/github.com/radovskyb/watcher@v1.0.7/watcher.go:517 +0x4be
github.com/radovskyb/watcher.(*Watcher).Start(0xc00011e080, 0x5f5e100)
        C:/Users/qknight/go/pkg/mod/github.com/radovskyb/watcher@v1.0.7/watcher.go:568 +0x158
main.fsNotifyWatchDocumentsDirectory({0xc000026640, 0x42})
        C:/Users/qknight/Desktop/Projects/pankat/cmd/pankat-server/fsNotify.go:96 +0xc5
created by main.main
        C:/Users/qknight/Desktop/Projects/pankat/cmd/pankat-server/main.go:35 +0x315

And this is the fix:

for name, recursive := range w.names {
    if recursive {
        list, err = w.listRecursive(name)
        if err != nil {
+           e, ok := err.(syscall.Errno)
+           if ok && e.Is(err) {
                if os.IsNotExist(err) {
                    w.mu.Unlock()
                    if name == err.(*os.PathError).Path {
                        w.Error <- ErrWatchedFileDeleted
                        w.RemoveRecursive(name)
                    }
                    w.mu.Lock()
                } else {
                    w.Error <- err
                }
+           }
        }
    } else {
        list, err = w.list(name)
        if err != nil {
+           e, ok := err.(syscall.Errno)
+           if ok && e.Is(err) {
                 if os.IsNotExist(err) {
                    w.mu.Unlock()
                    if name == err.(*os.PathError).Path {
                        w.Error <- ErrWatchedFileDeleted
                        w.Remove(name)
                    }
                    w.mu.Lock()
                } else {
                    w.Error <- err
                }
+           }
        }
    }

I'm not sure what this code does exactly and if my fix is correct but at least for me the watcher is now working according to specification!