Skip to content

Commit dce232c

Browse files
authored
fix(mediascanner): prevent indefinite hangs on stale network mounts (#549)
* fix(mediascanner): prevent indefinite hangs on stale network mounts Wrap filesystem operations (os.Stat, os.ReadDir, os.Lstat, filepath.EvalSymlinks) with context-aware timeouts to prevent the media scanner from hanging indefinitely on stale NFS/CIFS mounts. Introduce a directory cache (dirCache) for case-insensitive path lookups to reduce repeated filesystem calls during scanning. Also: - Handle SQLite UNIQUE constraint failures gracefully during indexing - Add phase constants (PhaseDiscovering, PhaseInitializing) replacing magic strings in status reporting - Switch GOPROXY from goproxy.io to direct - Fix test assertion for updated Total calculation formula * fix(mediascanner): resolve symlinks in fsutil test expectations EvalSymlinks resolves all symlinks in the path including those in the temp directory itself (e.g. /var -> /private/var on macOS, 8.3 short names on Windows). Compare against the resolved expected path instead of the raw t.TempDir() path.
1 parent 4c02bc1 commit dce232c

13 files changed

Lines changed: 680 additions & 63 deletions

File tree

Taskfile.dist.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ version: "3"
22

33
env:
44
GOTOOLCHAIN: auto
5-
GOPROXY: https://goproxy.io,direct
5+
GOPROXY: direct
66
APP_VERSION:
77
sh: echo -n "${APP_VERSION:-$(git rev-parse --short HEAD)-dev}"
88
UID:

pkg/api/methods/media.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,12 @@ func GenerateMediaDB(
275275
defer db.MediaDB.BackgroundOperationDone()
276276
total, err := mediascanner.NewNamesIndex(indexCtx, pl, cfg, systems, db, func(status mediascanner.IndexStatus) {
277277
var desc string
278-
switch status.Step {
279-
case 1:
278+
switch {
279+
case status.Phase == mediascanner.PhaseDiscovering:
280280
desc = "Finding media folders"
281-
case status.Total:
281+
case status.Phase == mediascanner.PhaseInitializing:
282+
desc = "Initializing database"
283+
case status.Step == status.Total:
282284
desc = "Writing database"
283285
default:
284286
system, err := systemdefs.GetSystem(status.SystemID)

pkg/api/methods/methods_test.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"database/sql"
2525
"errors"
2626
"testing"
27+
"time"
2728

2829
"github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models"
2930
"github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests"
@@ -578,8 +579,10 @@ func TestHandleGenerateMedia_SystemFiltering(t *testing.T) {
578579

579580
for _, tt := range tests {
580581
t.Run(tt.name, func(t *testing.T) {
581-
// Reset indexing status to prevent race conditions between parallel tests
582-
statusInstance.clear()
582+
// Cancel any leftover background goroutine and wait for it
583+
// to release the global statusInstance before starting.
584+
CancelIndexing()
585+
ClearIndexingStatus()
583586

584587
// Create mock environment
585588
mockPlatform := mocks.NewMockPlatform()
@@ -674,6 +677,14 @@ func TestHandleGenerateMedia_SystemFiltering(t *testing.T) {
674677
require.NoError(t, err)
675678
assert.Nil(t, result)
676679

680+
// Cancel the background indexing goroutine and wait for it
681+
// to finish so it doesn't leak into the next subtest.
682+
appState.StopService()
683+
require.Eventually(t, func() bool {
684+
return !statusInstance.isRunning()
685+
}, 5*time.Second, 10*time.Millisecond,
686+
"background indexing goroutine did not stop")
687+
677688
// Verify mock expectations were met
678689
mockMediaDB.AssertExpectations(t)
679690
mockPlatform.AssertExpectations(t)
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Zaparoo Core
2+
// Copyright (c) 2026 The Zaparoo Project Contributors.
3+
// SPDX-License-Identifier: GPL-3.0-or-later
4+
//
5+
// This file is part of Zaparoo Core.
6+
//
7+
// Zaparoo Core is free software: you can redistribute it and/or modify
8+
// it under the terms of the GNU General Public License as published by
9+
// the Free Software Foundation, either version 3 of the License, or
10+
// (at your option) any later version.
11+
//
12+
// Zaparoo Core is distributed in the hope that it will be useful,
13+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
// GNU General Public License for more details.
16+
//
17+
// You should have received a copy of the GNU General Public License
18+
// along with Zaparoo Core. If not, see <http://www.gnu.org/licenses/>.
19+
20+
package mediascanner
21+
22+
import (
23+
"context"
24+
"os"
25+
"runtime"
26+
"strings"
27+
)
28+
29+
// dirCache caches directory listings to avoid redundant filesystem operations
30+
// during path discovery. Not safe for concurrent use — intended to be created
31+
// and consumed within a single GetSystemPaths call.
32+
type dirCache struct {
33+
entries map[string][]os.DirEntry
34+
}
35+
36+
func newDirCache() *dirCache {
37+
return &dirCache{entries: make(map[string][]os.DirEntry)}
38+
}
39+
40+
// list returns the directory entries for the given path, reading from cache
41+
// if available or performing a readDirWithContext and caching the result.
42+
func (c *dirCache) list(ctx context.Context, path string) ([]os.DirEntry, error) {
43+
if cached, ok := c.entries[path]; ok {
44+
return cached, nil
45+
}
46+
entries, err := readDirWithContext(ctx, path)
47+
if err != nil {
48+
return nil, err
49+
}
50+
c.entries[path] = entries
51+
return entries, nil
52+
}
53+
54+
// findEntry does a case-insensitive lookup for name in the directory listing
55+
// for dirPath. On Linux, prefers an exact case match. Returns the actual
56+
// entry name, or empty string if not found.
57+
func (c *dirCache) findEntry(ctx context.Context, dirPath, name string) (string, error) {
58+
entries, err := c.list(ctx, dirPath)
59+
if err != nil {
60+
return "", err
61+
}
62+
63+
if runtime.GOOS == "linux" {
64+
for _, e := range entries {
65+
if e.Name() == name {
66+
return e.Name(), nil
67+
}
68+
}
69+
}
70+
71+
for _, e := range entries {
72+
if strings.EqualFold(e.Name(), name) {
73+
return e.Name(), nil
74+
}
75+
}
76+
77+
return "", nil
78+
}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// Zaparoo Core
2+
// Copyright (c) 2026 The Zaparoo Project Contributors.
3+
// SPDX-License-Identifier: GPL-3.0-or-later
4+
//
5+
// This file is part of Zaparoo Core.
6+
//
7+
// Zaparoo Core is free software: you can redistribute it and/or modify
8+
// it under the terms of the GNU General Public License as published by
9+
// the Free Software Foundation, either version 3 of the License, or
10+
// (at your option) any later version.
11+
//
12+
// Zaparoo Core is distributed in the hope that it will be useful,
13+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
// GNU General Public License for more details.
16+
//
17+
// You should have received a copy of the GNU General Public License
18+
// along with Zaparoo Core. If not, see <http://www.gnu.org/licenses/>.
19+
20+
package mediascanner
21+
22+
import (
23+
"context"
24+
"os"
25+
"path/filepath"
26+
"runtime"
27+
"testing"
28+
29+
"github.com/stretchr/testify/assert"
30+
"github.com/stretchr/testify/require"
31+
)
32+
33+
func TestDirCache_CachesResults(t *testing.T) {
34+
dir := t.TempDir()
35+
require.NoError(t, os.WriteFile(filepath.Join(dir, "file.txt"), []byte("x"), 0o600))
36+
37+
cache := newDirCache()
38+
ctx := context.Background()
39+
40+
entries1, err := cache.list(ctx, dir)
41+
require.NoError(t, err)
42+
require.Len(t, entries1, 1)
43+
44+
// Add another file — cached result should still show 1 entry
45+
require.NoError(t, os.WriteFile(filepath.Join(dir, "file2.txt"), []byte("y"), 0o600))
46+
47+
entries2, err := cache.list(ctx, dir)
48+
require.NoError(t, err)
49+
assert.Len(t, entries2, 1, "should return cached result, not re-read directory")
50+
}
51+
52+
func TestDirCache_FindEntry_ExactMatch(t *testing.T) {
53+
dir := t.TempDir()
54+
require.NoError(t, os.Mkdir(filepath.Join(dir, "NES"), 0o750))
55+
56+
cache := newDirCache()
57+
ctx := context.Background()
58+
59+
name, err := cache.findEntry(ctx, dir, "NES")
60+
require.NoError(t, err)
61+
assert.Equal(t, "NES", name)
62+
}
63+
64+
func TestDirCache_FindEntry_CaseInsensitive(t *testing.T) {
65+
dir := t.TempDir()
66+
require.NoError(t, os.Mkdir(filepath.Join(dir, "SNES"), 0o750))
67+
68+
cache := newDirCache()
69+
ctx := context.Background()
70+
71+
name, err := cache.findEntry(ctx, dir, "snes")
72+
require.NoError(t, err)
73+
assert.Equal(t, "SNES", name)
74+
}
75+
76+
func TestDirCache_FindEntry_PrefersExactOnLinux(t *testing.T) {
77+
if runtime.GOOS != "linux" {
78+
t.Skip("exact-match preference only applies on Linux")
79+
}
80+
81+
dir := t.TempDir()
82+
// Create both cases — Linux allows this on case-sensitive filesystems
83+
require.NoError(t, os.Mkdir(filepath.Join(dir, "Games"), 0o750))
84+
require.NoError(t, os.Mkdir(filepath.Join(dir, "games"), 0o750))
85+
86+
cache := newDirCache()
87+
ctx := context.Background()
88+
89+
name, err := cache.findEntry(ctx, dir, "Games")
90+
require.NoError(t, err)
91+
assert.Equal(t, "Games", name)
92+
93+
name, err = cache.findEntry(ctx, dir, "games")
94+
require.NoError(t, err)
95+
assert.Equal(t, "games", name)
96+
}
97+
98+
func TestDirCache_FindEntry_NotFound(t *testing.T) {
99+
dir := t.TempDir()
100+
require.NoError(t, os.Mkdir(filepath.Join(dir, "NES"), 0o750))
101+
102+
cache := newDirCache()
103+
ctx := context.Background()
104+
105+
name, err := cache.findEntry(ctx, dir, "GENESIS")
106+
require.NoError(t, err)
107+
assert.Empty(t, name)
108+
}
109+
110+
func TestDirCache_ContextCancellation(t *testing.T) {
111+
ctx, cancel := context.WithCancel(context.Background())
112+
cancel()
113+
114+
cache := newDirCache()
115+
116+
_, err := cache.list(ctx, t.TempDir())
117+
require.Error(t, err)
118+
require.ErrorIs(t, err, context.Canceled)
119+
120+
_, err = cache.findEntry(ctx, t.TempDir(), "anything")
121+
require.Error(t, err)
122+
require.ErrorIs(t, err, context.Canceled)
123+
}

pkg/database/mediascanner/findpath_fuzz_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package mediascanner
2121

2222
import (
23+
"context"
2324
"os"
2425
"path/filepath"
2526
"testing"
@@ -63,7 +64,8 @@ func FuzzFindPath(f *testing.F) {
6364
fullPath := filepath.Join(tmpDir, pathFragment)
6465

6566
// EXECUTE: Call FindPath
66-
result, err := FindPath(fullPath)
67+
ctx := context.Background()
68+
result, err := FindPath(ctx, fullPath)
6769

6870
// INVARIANT CHECKS
6971
if err == nil {
@@ -78,7 +80,7 @@ func FuzzFindPath(f *testing.F) {
7880
}
7981

8082
// INVARIANT 4: Idempotency - calling FindPath on result should return same result
81-
result2, err2 := FindPath(result)
83+
result2, err2 := FindPath(ctx, result)
8284
if err2 != nil {
8385
t.Errorf("FindPath failed on its own output: %s (error: %v)", result, err2)
8486
} else if result != result2 {

0 commit comments

Comments
 (0)