Skip to content

Commit 92b4731

Browse files
authored
fix(config): make DefaultPaths() thread-safe with sync.Once (#98)
1 parent 0de0005 commit 92b4731

3 files changed

Lines changed: 116 additions & 9 deletions

File tree

.github/workflows/build.yml

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,57 @@ jobs:
9696
fi
9797
echo "✅ go.mod and go.sum are tidy"
9898
99+
race-detection:
100+
name: Race Detection
101+
runs-on: ubuntu-latest
102+
steps:
103+
- name: Checkout code
104+
uses: actions/checkout@v4
105+
106+
- name: Set up Go
107+
uses: actions/setup-go@v5
108+
with:
109+
go-version: '1.23'
110+
111+
- name: Get dependencies
112+
run: go mod download
113+
114+
- name: Run tests with race detector
115+
run: |
116+
echo "Running tests with Go race detector enabled..."
117+
echo "This detects data races in concurrent code (requires cgo)"
118+
go test -race -v ./src/... 2>&1 | tee race-test-output.log
119+
TEST_EXIT_CODE=${PIPESTATUS[0]}
120+
exit $TEST_EXIT_CODE
121+
122+
- name: Generate race detection summary
123+
if: always()
124+
run: |
125+
echo "## Race Detection Results" >> $GITHUB_STEP_SUMMARY
126+
echo "" >> $GITHUB_STEP_SUMMARY
127+
128+
if grep -q "WARNING: DATA RACE" race-test-output.log 2>/dev/null; then
129+
echo "❌ **Data races detected!**" >> $GITHUB_STEP_SUMMARY
130+
echo "" >> $GITHUB_STEP_SUMMARY
131+
echo "### Race Details" >> $GITHUB_STEP_SUMMARY
132+
echo '```' >> $GITHUB_STEP_SUMMARY
133+
grep -A 50 "WARNING: DATA RACE" race-test-output.log >> $GITHUB_STEP_SUMMARY
134+
echo '```' >> $GITHUB_STEP_SUMMARY
135+
else
136+
echo "✅ **No data races detected**" >> $GITHUB_STEP_SUMMARY
137+
fi
138+
139+
echo "" >> $GITHUB_STEP_SUMMARY
140+
echo "<details>" >> $GITHUB_STEP_SUMMARY
141+
echo "<summary>Full Test Output</summary>" >> $GITHUB_STEP_SUMMARY
142+
echo "" >> $GITHUB_STEP_SUMMARY
143+
echo '```' >> $GITHUB_STEP_SUMMARY
144+
cat race-test-output.log >> $GITHUB_STEP_SUMMARY
145+
echo '```' >> $GITHUB_STEP_SUMMARY
146+
echo "</details>" >> $GITHUB_STEP_SUMMARY
147+
99148
build:
100-
needs: [golangci-lint, go-mod]
149+
needs: [golangci-lint, go-mod, race-detection]
101150
name: Build ${{ matrix.platform }} on ${{ matrix.os }}
102151
runs-on: ${{ matrix.os }}
103152
strategy:

src/internal/config/paths.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"path/filepath"
77
"runtime"
8+
"sync"
89

910
"github.com/dtvem/dtvem/src/internal/constants"
1011
)
@@ -17,13 +18,17 @@ type Paths struct {
1718
Config string // Config directory (~/.dtvem/config)
1819
}
1920

20-
var defaultPaths *Paths
21+
var (
22+
defaultPaths *Paths
23+
pathsOnce sync.Once
24+
)
2125

22-
// DefaultPaths returns the default dtvem paths
26+
// DefaultPaths returns the default dtvem paths.
27+
// This function is thread-safe and guarantees single initialization.
2328
func DefaultPaths() *Paths {
24-
if defaultPaths == nil {
29+
pathsOnce.Do(func() {
2530
defaultPaths = initPaths()
26-
}
31+
})
2732
return defaultPaths
2833
}
2934

src/internal/config/paths_test.go

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"path/filepath"
66
"runtime"
77
"strings"
8+
"sync"
89
"testing"
910
)
1011

@@ -172,6 +173,13 @@ func TestShimPath(t *testing.T) {
172173
}
173174
}
174175

176+
// resetPathsForTesting resets the paths singleton for testing purposes.
177+
// This allows tests to verify behavior with different environment configurations.
178+
func resetPathsForTesting() {
179+
defaultPaths = nil
180+
pathsOnce = sync.Once{}
181+
}
182+
175183
func TestGetRootDir_WithEnvironmentVariable(t *testing.T) {
176184
// Save original environment
177185
originalRoot := os.Getenv("DTVEM_ROOT")
@@ -181,16 +189,16 @@ func TestGetRootDir_WithEnvironmentVariable(t *testing.T) {
181189
} else {
182190
_ = os.Unsetenv("DTVEM_ROOT")
183191
}
184-
// Reset defaultPaths so it reinitializes
185-
defaultPaths = nil
192+
// Reset paths so it reinitializes
193+
resetPathsForTesting()
186194
}()
187195

188196
// Set custom DTVEM_ROOT
189197
customRoot := "/custom/dtvem/path"
190198
_ = os.Setenv("DTVEM_ROOT", customRoot)
191199

192-
// Reset defaultPaths to force reinitialization
193-
defaultPaths = nil
200+
// Reset paths to force reinitialization
201+
resetPathsForTesting()
194202

195203
// Test that getRootDir respects DTVEM_ROOT
196204
result := getRootDir()
@@ -232,3 +240,48 @@ func TestLocalConfigPath(t *testing.T) {
232240
t.Errorf("LocalConfigPath() = %q, should end with %q", result, RuntimesFileName)
233241
}
234242
}
243+
244+
func TestDefaultPaths_ConcurrentAccess(t *testing.T) {
245+
// Reset to ensure clean state
246+
resetPathsForTesting()
247+
defer resetPathsForTesting()
248+
249+
const goroutines = 100
250+
var wg sync.WaitGroup
251+
wg.Add(goroutines)
252+
253+
// Channel to collect results
254+
results := make(chan *Paths, goroutines)
255+
256+
// Launch multiple goroutines to call DefaultPaths concurrently
257+
for i := 0; i < goroutines; i++ {
258+
go func() {
259+
defer wg.Done()
260+
results <- DefaultPaths()
261+
}()
262+
}
263+
264+
wg.Wait()
265+
close(results)
266+
267+
// Collect all results
268+
var first *Paths
269+
for paths := range results {
270+
if first == nil {
271+
first = paths
272+
} else {
273+
// All goroutines should receive the same pointer
274+
if paths != first {
275+
t.Errorf("DefaultPaths() returned different pointers: %p vs %p", first, paths)
276+
}
277+
}
278+
}
279+
280+
// Verify the paths are valid
281+
if first == nil {
282+
t.Fatal("DefaultPaths() returned nil")
283+
}
284+
if first.Root == "" {
285+
t.Error("Root path is empty")
286+
}
287+
}

0 commit comments

Comments
 (0)