From af0ebf6d5106ef14e30f001762daad43e0ce14af Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Fri, 15 Aug 2025 09:00:50 +0000 Subject: [PATCH] test: prevent data race detection in TestActionCache (#858) Data race is being flagged because a goroutine is currently writing tar data into `atar`, and `assert.NotEmpty(atar)` performs internal structure reflection into the returned `io.PipeWriter` and violates its internal synchronization primitives. This assertion doesn't seem to add any value to the test compared to just reading the pipe, so it has been removed. Data race details (abbreviated): ``` ================== WARNING: DATA RACE Write at 0x00c00041fbc0 by goroutine 55: sync/atomic.CompareAndSwapInt32() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/runtime/race_amd64.s:361 +0xb sync/atomic.CompareAndSwapInt32() :1 +0x18 sync.(*Mutex).Lock() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/sync/mutex.go:46 +0x28 io.(*pipe).write() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/io/pipe.go:81 +0x9e io.(*PipeWriter).Write() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/io/pipe.go:161 +0x46 archive/tar.(*Writer).Flush() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/archive/tar/writer.go:59 +0xcc archive/tar.(*Writer).WriteHeader() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/archive/tar/writer.go:71 +0x46 code.forgejo.org/forgejo/runner/v9/act/runner.actionCacheCopyFileOrDir() /home/mfenniak/Dev/forgejo-runner/act/runner/action_cache.go:203 +0xabd code.forgejo.org/forgejo/runner/v9/act/runner.GoGitActionCache.GetTarArchive.func2.1() /home/mfenniak/Dev/forgejo-runner/act/runner/action_cache.go:154 +0xa5 ... Previous read at 0x00c00041fbc0 by goroutine 9: reflect.typedmemmove() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/runtime/mbarrier.go:213 +0x0 reflect.packEface() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/reflect/value.go:136 +0xc5 reflect.valueInterface() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/reflect/value.go:1513 +0x179 reflect.Value.Interface() /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/reflect/value.go:1484 +0x106 github.com/stretchr/testify/assert.isEmpty() /home/mfenniak/go/pkg/mod/github.com/stretchr/testify@v1.10.0/assert/assertions.go:735 +0x7f github.com/stretchr/testify/assert.NotEmpty() /home/mfenniak/go/pkg/mod/github.com/stretchr/testify@v1.10.0/assert/assertions.go:769 +0x56 github.com/stretchr/testify/assert.(*Assertions).NotEmpty() /home/mfenniak/go/pkg/mod/github.com/stretchr/testify@v1.10.0/assert/assertion_forward.go:1175 +0xb1 code.forgejo.org/forgejo/runner/v9/act/runner.TestActionCache.func1() /home/mfenniak/Dev/forgejo-runner/act/runner/action_cache_test.go:60 +0x21e ================== ``` - other - [PR](https://code.forgejo.org/forgejo/runner/pulls/858): test: prevent data race detection in TestActionCache [skip cascade] Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/858 Reviewed-by: Gusted Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- act/runner/action_cache_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/act/runner/action_cache_test.go b/act/runner/action_cache_test.go index 765eef0d..16518856 100644 --- a/act/runner/action_cache_test.go +++ b/act/runner/action_cache_test.go @@ -57,9 +57,10 @@ func TestActionCache(t *testing.T) { return } atar, err := cache.GetTarArchive(ctx, c.CacheDir, sha, "js") - if !a.NoError(err) || !a.NotEmpty(atar) { + if !a.NoError(err) { return } + defer atar.Close() mytar := tar.NewReader(atar) th, err := mytar.Next() if !a.NoError(err) || !a.NotEqual(0, th.Size) {