[gitsync] Workaround for race
Not a race in real code; just add a mutex in the test to avoid the
problem.
Change-Id: Ib946522eee57b9ec96ac7e2b199a53c8b9174799
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/236303
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
diff --git a/gitsync/go/watcher/watcher_test.go b/gitsync/go/watcher/watcher_test.go
index 8475389..4fefa63 100644
--- a/gitsync/go/watcher/watcher_test.go
+++ b/gitsync/go/watcher/watcher_test.go
@@ -6,6 +6,7 @@
"fmt"
"io/ioutil"
"strings"
+ "sync"
"testing"
"time"
@@ -59,8 +60,15 @@
totalIngested += numNew
assert.Equal(t, totalIngested, len(ri.Commits))
}
+ // There's a data race between the mocked GitStore's use of reflection
+ // and the lock/unlock of a mutex in Context.Err(). We add a mutex here
+ // to avoid that problem.
+ var ctxMtx sync.Mutex
process := func(ctx context.Context, cb *commitBatch) error {
- if err := ri.gitstore.Put(ctx, cb.commits); err != nil {
+ ctxMtx.Lock()
+ err := ri.gitstore.Put(ctx, cb.commits)
+ ctxMtx.Unlock()
+ if err != nil {
return err
}
for _, c := range cb.commits {
@@ -115,7 +123,11 @@
// Ensure that the context gets canceled if ingestion fails.
err = ri.processCommits(ctx, process, func(ctx context.Context, ch chan<- *commitBatch) error {
for i := 5; i < 10; i++ {
- if err := ctx.Err(); err != nil {
+ // See the above comment about mocks, reflect, and race.
+ ctxMtx.Lock()
+ err := ctx.Err()
+ ctxMtx.Unlock()
+ if err != nil {
return err
}
hashes := make([]string, 0, i)
