tarfs: fix a data race condition

WARNING: DATA RACE Write at 0x00c000178428 by goroutine 27: github.com/containerd/nydus-snapshotter/pkg/remote/remotes/docker.(*httpReadSeeker).Close() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/remote/remotes/docker/httpreadseeker.go:87 +0x57 github.com/containerd/nydus-snapshotter/pkg/tarfs.(*Manager).blobProcess.func2.1() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs.go:339 +0x48 runtime.deferreturn() /opt/hostedtoolcache/go/1.20.1/x64/src/runtime/panic.go:476 +0x32 github.com/containerd/nydus-snapshotter/pkg/tarfs.(*Manager).blobProcess.func3() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs.go:394 +0x71 Previous read at 0x00c000178428 by goroutine 40: github.com/containerd/nydus-snapshotter/pkg/remote/remotes/docker.(*httpReadSeeker).Read() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/remote/remotes/docker/httpreadseeker.go:48 +0x68 bufio.(*Reader).Read() /opt/hostedtoolcache/go/1.20.1/x64/src/bufio/bufio.go:223 +0x2c3 github.com/containerd/containerd/archive/compression.(*bufferedReader).Read() /home/runner/go/pkg/mod/github.com/containerd/containerd@v1.7.0/archive/compression/compression.go:113 +0xa4 io.copyBuffer() /opt/hostedtoolcache/go/1.20.1/x64/src/io/io.go:427 +0x28d io.Copy() /opt/hostedtoolcache/go/1.20.1/x64/src/io/io.go:386 +0x88 os.genericReadFrom() /opt/hostedtoolcache/go/1.20.1/x64/src/os/file.go:161 +0x34 os.(*File).ReadFrom() /opt/hostedtoolcache/go/1.20.1/x64/src/os/file.go:155 +0x324 io.copyBuffer() /opt/hostedtoolcache/go/1.20.1/x64/src/io/io.go:413 +0x1c5 io.Copy() /opt/hostedtoolcache/go/1.20.1/x64/src/io/io.go:386 +0x84 os/exec.(*Cmd).childStdin.func1() /opt/hostedtoolcache/go/1.20.1/x64/src/os/exec/exec.go:511 +0x45 os/exec.(*Cmd).Start.func2() /opt/hostedtoolcache/go/1.20.1/x64/src/os/exec/exec.go:717 +0x42 os/exec.(*Cmd).Start.func3() /opt/hostedtoolcache/go/1.20.1/x64/src/os/exec/exec.go:729 +0x47 Goroutine 27 (running) created at: github.com/containerd/nydus-snapshotter/pkg/tarfs.(*Manager).blobProcess() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs.go:393 +0x9dd github.com/containerd/nydus-snapshotter/pkg/tarfs.(*Manager).PrepareLayer() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs.go:465 +0x444 github.com/containerd/nydus-snapshotter/pkg/tarfs.TestPrepareLayer() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs_test.go:33 +0x188 testing.tRunner() /opt/hostedtoolcache/go/1.20.1/x64/src/testing/testing.go:1576 +0x216 testing.(*T).Run.func1() /opt/hostedtoolcache/go/1.20.1/x64/src/testing/testing.go:1629 +0x47 Goroutine 40 (finished) created at: os/exec.(*Cmd).Start() /opt/hostedtoolcache/go/1.20.1/x64/src/os/exec/exec.go:716 +0xf8e github.com/containerd/containerd/archive/compression.cmdStream() /home/runner/go/pkg/mod/github.com/containerd/containerd@v1.7.0/archive/compression/compression.go:284 +0x36f github.com/containerd/containerd/archive/compression.gzipDecompress() /home/runner/go/pkg/mod/github.com/containerd/containerd@v1.7.0/archive/compression/compression.go:272 +0x152 github.com/containerd/containerd/archive/compression.DecompressStream() /home/runner/go/pkg/mod/github.com/containerd/containerd@v1.7.0/archive/compression/compression.go:203 +0x3e4 github.com/containerd/nydus-snapshotter/pkg/tarfs.(*Manager).blobProcess.func2() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs.go:341 +0x1b1 github.com/containerd/nydus-snapshotter/pkg/tarfs.(*Manager).blobProcess.func3() /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/tarfs/tarfs.go:394 +0x71 ================== testing.go:1446: race detected during execution of test Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
containerd · Nov 20, 2023 · ea0b54c · ea0b54c
1 parent f0323b2
commit ea0b54c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 14 deletions.
diff --git a/pkg/tarfs/tarfs.go b/pkg/tarfs/tarfs.go
@@ -384,16 +384,19 @@ func (t *Manager) blobProcess(ctx context.Context, snapshotID, ref string,
 		return epilog(err, "get blob stream for layer")
 	}
 
-	if retry {
-		// Download and convert layer content in synchronous mode when retry for error recovering
-		err = process(rc, remote)
-	} else {
-		// Download and convert layer content in background.
-		// Will retry when the content is actually needed if the background process failed.
-		go func() {
-			_ = process(rc, remote)
-		}()
-	}
+	err = process(rc, remote)
+	/*
+		if retry {
+			// Download and convert layer content in synchronous mode when retry for error recovering
+			err = process(rc, remote)
+		} else {
+			// Download and convert layer content in background.
+			// Will retry when the content is actually needed if the background process failed.
+			go func() {
+				_ = process(rc, remote)
+			}()
+		}
+	*/
 
 	return err
 }
@@ -999,15 +1002,15 @@ func (t *Manager) CheckTarfsHintAnnotation(ctx context.Context, ref string, mani
 	if err != nil {
 		return false, err
 	}
-	remote := remote.New(keyChain, t.insecure)
+	remoteFactory := remote.New(keyChain, t.insecure)
 
 	handle := func() (bool, error) {
 		if tarfsHint, ok := t.tarfsHintCache.Get(ref); ok {
 			return tarfsHint.(bool), nil
 		}
 
 		if _, err, _ := t.sg.Do(ref, func() (interface{}, error) {
-			err := t.fetchImageInfo(ctx, remote, ref, manifestDigest)
+			err := t.fetchImageInfo(ctx, remoteFactory, ref, manifestDigest)
 			return nil, err
 		}); err != nil {
 			return false, err
@@ -1021,7 +1024,8 @@ func (t *Manager) CheckTarfsHintAnnotation(ctx context.Context, ref string, mani
 	}
 
 	tarfsHint, err := handle()
-	if err != nil && remote.RetryWithPlainHTTP(ref, err) {
+	if err != nil && remoteFactory.RetryWithPlainHTTP(ref, err) {
+		remoteFactory = remote.New(keyChain, t.insecure)
 		tarfsHint, err = handle()
 	}
 	return tarfsHint, err

diff --git a/pkg/tarfs/tarfs_test.go b/pkg/tarfs/tarfs_test.go
@@ -26,7 +26,7 @@ const (
 // TODO: add unit test for MergeLayers, ExportBlockData, MountErofs, RemountErofs, UmountTarErofs, DetachLayer, RecoverSnapshoInfo, RecoverRafsInstance
 
 func TestPrepareLayer(t *testing.T) {
-	manager := NewManager(true, true, "/tmp/tarfs", "/usr/bin/nydus-image", 4)
+	manager := NewManager(true, true, t.TempDir(), "/usr/bin/nydus-image", 4)
 	manifestDigest, err := digest.Parse(BusyboxManifestDigest)
 	assert.Assert(t, err)
 	layerDigest, err := digest.Parse(BusyboxLayerDigest)