From e42418db77a24a99f237427c0d574cae870da156 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 27 Sep 2026 15:25:19 -0400 Subject: [PATCH] fix(windows): avoid filepath.Rel hang on UNC share roots On Windows, filepath.Rel never returns when the base is a UNC share root (\\host\share) and the target is the same root with a trailing separator (golang/go#79784, fixed only in Go 1.28 by CL 786100). A user on Windows Server with MusicFolder='\\server\Music' (discussion #6232) sent CPU profiles showing 96% of CPU time in filepath.Rel, called from fromArtistFolder <- resolveArtistFolderPattern <- resolver.resolveArtist. For an album directly under the library root, loadArtistFolder's filepath.Dir returns '\\server\Music\', so the artwork worker spins forever and, because Worker.drain waits for the whole batch, the artist artwork pool stops. Add utils.RelPath, which normalizes a UNC volume root followed only by separators to the volume plus one separator before calling filepath.Rel. Both sides then compare equal and Rel returns '.' before reaching the loop. A bare '\\host\share' target under a deeper base now also gives '..' instead of an error. Drive-letter paths are left unchanged, and on non-Windows systems the helper behaves exactly like filepath.Rel. Use it at every production filepath.Rel call site: artist folder artwork, scanner library-relative paths, the watcher, playlist path resolution and PlaylistsPath matching, and the cache spread FS. Windows-only tests cover the UNC cases, with a timeout so a regression fails instead of hanging CI. Run under Wine, 6 of them fail with plain filepath.Rel (4 by timeout) and all pass with RelPath. The helper can be removed after moving to Go 1.28. --- core/artwork/folders_artist.go | 2 +- core/playlists/parse_m3u.go | 3 ++- core/playlists/playlists.go | 3 ++- scanner/scanner.go | 3 ++- scanner/watcher.go | 3 ++- utils/cache/spread_fs.go | 3 ++- utils/files.go | 13 +++++++++++ utils/files_test.go | 23 +++++++++++++++++++ utils/files_windows_test.go | 40 ++++++++++++++++++++++++++++++++++ 9 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 utils/files_windows_test.go diff --git a/core/artwork/folders_artist.go b/core/artwork/folders_artist.go index 3403935db..40972f71d 100644 --- a/core/artwork/folders_artist.go +++ b/core/artwork/folders_artist.go @@ -30,7 +30,7 @@ func fromArtistFolder(ctx context.Context, libFS fs.FS, libPath, artistFolder, p if libFS == nil { return nil, "", fmt.Errorf("artist folder lookup unavailable") } - rel, err := filepath.Rel(libPath, artistFolder) + rel, err := utils.RelPath(libPath, artistFolder) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return nil, "", fmt.Errorf(`artist folder '%s' is outside library '%s'`, artistFolder, libPath) } diff --git a/core/playlists/parse_m3u.go b/core/playlists/parse_m3u.go index 9610e9dbb..9ca1c9b6c 100644 --- a/core/playlists/parse_m3u.go +++ b/core/playlists/parse_m3u.go @@ -15,6 +15,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/slice" "golang.org/x/text/unicode/norm" ) @@ -148,7 +149,7 @@ func (r pathResolution) ToQualifiedString() (string, error) { if !r.valid { return "", fmt.Errorf("invalid path resolution") } - relativePath, err := filepath.Rel(r.libraryPath, r.absolutePath) + relativePath, err := utils.RelPath(r.libraryPath, r.absolutePath) if err != nil { return "", err } diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index c9bc03b97..8e7ccf93c 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -15,6 +15,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/utils" ) type Playlists interface { @@ -77,7 +78,7 @@ func InPath(folder model.Folder) bool { if conf.Server.PlaylistsPath == "" { return true } - rel, _ := filepath.Rel(folder.LibraryPath, folder.AbsolutePath()) + rel, _ := utils.RelPath(folder.LibraryPath, folder.AbsolutePath()) for path := range strings.SplitSeq(conf.Server.PlaylistsPath, string(filepath.ListSeparator)) { if match, _ := doublestar.Match(path, rel); match { return true diff --git a/scanner/scanner.go b/scanner/scanner.go index cd2fe3c8d..f156d15fd 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -15,6 +15,7 @@ import ( "github.com/navidrome/navidrome/core/playlists" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/run" "github.com/navidrome/navidrome/utils/slice" ) @@ -65,7 +66,7 @@ func libraryRelativePath(libPath, folderPath string) string { if err != nil { return folderPath } - rel, err := filepath.Rel(absLib, folderPath) + rel, err := utils.RelPath(absLib, folderPath) if err != nil || !filepath.IsLocal(rel) { return folderPath } diff --git a/scanner/watcher.go b/scanner/watcher.go index 1ac5468f0..3c0fbdbe7 100644 --- a/scanner/watcher.go +++ b/scanner/watcher.go @@ -13,6 +13,7 @@ import ( "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/singleton" ) @@ -245,7 +246,7 @@ func (w *watcher) processLibraryEvents(ctx context.Context, lib *model.Library, log.Debug(ctx, "Watcher stopped due to context cancellation", "libraryID", lib.ID, "name", lib.Name) return nil case path := <-events: - path, err := filepath.Rel(absLibPath, path) + path, err := utils.RelPath(absLibPath, path) if err != nil { log.Error(ctx, "Error getting relative path", "libraryID", lib.ID, "absolutePath", absLibPath, "path", path, err) continue diff --git a/utils/cache/spread_fs.go b/utils/cache/spread_fs.go index 11801b324..3ebd00e1a 100644 --- a/utils/cache/spread_fs.go +++ b/utils/cache/spread_fs.go @@ -12,6 +12,7 @@ import ( "github.com/djherbis/fscache" "github.com/djherbis/stream" "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/utils" ) const completeMarkerSuffix = ".complete" @@ -97,7 +98,7 @@ func (sfs *spreadFS) walkDataFiles(visit func(absoluteFilePath string)) error { log.Error("Error loading cache", "dir", sfs.root, err) return nil } - path, err := filepath.Rel(sfs.root, absoluteFilePath) + path, err := utils.RelPath(sfs.root, absoluteFilePath) if err != nil { return nil //nolint:nilerr } diff --git a/utils/files.go b/utils/files.go index 2fce307ea..8ef522e5e 100644 --- a/utils/files.go +++ b/utils/files.go @@ -40,3 +40,16 @@ func FileExists(path string) bool { _, err := os.Stat(path) return err == nil || !os.IsNotExist(err) } + +// RelPath is filepath.Rel without the UNC share root hang (golang/go#79784). Remove it after moving to Go 1.28. +func RelPath(basePath, targPath string) (string, error) { + return filepath.Rel(normalizeUNCRoot(basePath), normalizeUNCRoot(targPath)) +} + +func normalizeUNCRoot(p string) string { + vol := filepath.VolumeName(p) + if len(vol) > 2 && strings.Trim(p[len(vol):], `\/`) == "" { + return vol + string(filepath.Separator) + } + return p +} diff --git a/utils/files_test.go b/utils/files_test.go index c6e578f05..04b4d4e0b 100644 --- a/utils/files_test.go +++ b/utils/files_test.go @@ -223,3 +223,26 @@ var _ = Describe("FileExists", func() { Expect(result).To(Or(BeTrue(), BeFalse())) // Should not panic }) }) + +var _ = Describe("RelPath", func() { + DescribeTable("returns the same result as filepath.Rel", + func(base, target string) { + base, target = filepath.FromSlash(base), filepath.FromSlash(target) + expected, expectedErr := filepath.Rel(base, target) + rel, err := utils.RelPath(base, target) + Expect(rel).To(Equal(expected)) + if expectedErr == nil { + Expect(err).ToNot(HaveOccurred()) + } else { + Expect(err).To(MatchError(expectedErr.Error())) + } + }, + Entry("same path", "/music", "/music"), + Entry("same path with trailing separator", "/music", "/music/"), + Entry("child path", "/music", "/music/artist/album"), + Entry("sibling path", "/music/a", "/music/b"), + Entry("parent path", "/music/artist", "/music"), + Entry("relative paths", "music", "music/artist"), + Entry("absolute and relative paths", "/music", "music"), + ) +}) diff --git a/utils/files_windows_test.go b/utils/files_windows_test.go new file mode 100644 index 000000000..419291b98 --- /dev/null +++ b/utils/files_windows_test.go @@ -0,0 +1,40 @@ +package utils_test + +import ( + "time" + + "github.com/navidrome/navidrome/utils" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("RelPath on Windows", func() { + type result struct { + rel string + err error + } + + DescribeTable("handles UNC share roots without hanging", + func(base, target, expected string) { + done := make(chan result, 1) + go func() { + rel, err := utils.RelPath(base, target) + done <- result{rel, err} + }() + var res result + Eventually(done).WithTimeout(5 * time.Second).Should(Receive(&res)) + Expect(res.err).ToNot(HaveOccurred()) + Expect(res.rel).To(Equal(expected)) + }, + Entry("root and root with trailing separator", `\\server\Music`, `\\server\Music\`, "."), + Entry("root with trailing separator and root", `\\server\Music\`, `\\server\Music`, "."), + Entry("root and root with many separators", `\\server\Music`, `\\server\Music\\`, "."), + Entry("root with forward slashes", `//server/Music`, `//server/Music/`, "."), + Entry("root and child folder", `\\server\Music`, `\\server\Music\Artist`, "Artist"), + Entry("root with trailing separator and child folder", `\\server\Music\`, `\\server\Music\Artist\Album`, `Artist\Album`), + Entry("child folder and root", `\\server\Music\Artist`, `\\server\Music\`, ".."), + Entry("child folder and root without trailing separator", `\\server\Music\Artist`, `\\server\Music`, ".."), + Entry("root and root with different case", `\\server\Music`, `\\SERVER\music\`, "."), + Entry("drive root and child folder", `C:\`, `C:\Music`, "Music"), + ) +})