From 5b8d3d94b0bf21259f0a63e07f5398b555f98745 Mon Sep 17 00:00:00 2001
From: Kyle Tarplee <kmtarplee@ieee.org>
Date: Mon, 25 Sep 2023 22:43:43 -0400
Subject: [PATCH] fix: Avoid a copy of sync.Mutex in Repository (#603)

Closes #600
Signed-off-by: Kyle M. Tarplee <kmtarplee@ieee.org>
---
 registry/remote/repository.go      | 32 +++++++++++++++++++-----------
 registry/remote/repository_test.go | 21 ++++++++++++++++++++
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/registry/remote/repository.go b/registry/remote/repository.go
index 337b4115..fc4f6bf6 100644
--- a/registry/remote/repository.go
+++ b/registry/remote/repository.go
@@ -147,7 +147,7 @@ type Repository struct {
 	//   - https://www.rfc-editor.org/rfc/rfc7234#section-5.5
 	HandleWarning func(warning Warning)
 
-	// NOTE: Must keep fields in sync with newRepositoryWithOptions function.
+	// NOTE: Must keep fields in sync with clone().
 
 	// referrersState represents that if the repository supports Referrers API.
 	// default: referrersStateUnknown
@@ -186,16 +186,24 @@ func newRepositoryWithOptions(ref registry.Reference, opts *RepositoryOptions) (
 	if err := ref.ValidateRepository(); err != nil {
 		return nil, err
 	}
+	repo := (*Repository)(opts).clone()
+	repo.Reference = ref
+	return repo, nil
+}
+
+// clone makes a copy of the Repository being careful not to copy non-copyable fields (sync.Mutex and syncutil.Pool types)
+func (r *Repository) clone() *Repository {
 	return &Repository{
-		Client:               opts.Client,
-		Reference:            ref,
-		PlainHTTP:            opts.PlainHTTP,
-		SkipReferrersGC:      opts.SkipReferrersGC,
-		ManifestMediaTypes:   slices.Clone(opts.ManifestMediaTypes),
-		TagListPageSize:      opts.TagListPageSize,
-		ReferrerListPageSize: opts.ReferrerListPageSize,
-		MaxMetadataBytes:     opts.MaxMetadataBytes,
-	}, nil
+		Client:               r.Client,
+		Reference:            r.Reference,
+		PlainHTTP:            r.PlainHTTP,
+		ManifestMediaTypes:   slices.Clone(r.ManifestMediaTypes),
+		TagListPageSize:      r.TagListPageSize,
+		ReferrerListPageSize: r.ReferrerListPageSize,
+		MaxMetadataBytes:     r.MaxMetadataBytes,
+		SkipReferrersGC:      r.SkipReferrersGC,
+		HandleWarning:        r.HandleWarning,
+	}
 }
 
 // SetReferrersCapability indicates the Referrers API capability of the remote
@@ -803,10 +811,10 @@ func (s *blobStore) Mount(ctx context.Context, desc ocispec.Descriptor, fromRepo
 // sibling returns a blob store for another repository in the same
 // registry.
 func (s *blobStore) sibling(otherRepoName string) *blobStore {
-	otherRepo := *s.repo
+	otherRepo := s.repo.clone()
 	otherRepo.Reference.Repository = otherRepoName
 	return &blobStore{
-		repo: &otherRepo,
+		repo: otherRepo,
 	}
 }
 
diff --git a/registry/remote/repository_test.go b/registry/remote/repository_test.go
index 3e3ee61a..b6772cbd 100644
--- a/registry/remote/repository_test.go
+++ b/registry/remote/repository_test.go
@@ -7447,3 +7447,24 @@ func TestRepository_do(t *testing.T) {
 		t.Errorf("Repository.do() = %v, want %v", gotWarnings, wantWarnings)
 	}
 }
+
+func TestRepository_clone(t *testing.T) {
+	repo, err := NewRepository("localhost:1234/repo/image")
+	if err != nil {
+		t.Fatalf("invalid repository: %v", err)
+	}
+
+	crepo := repo.clone()
+
+	if repo.Reference != crepo.Reference {
+		t.Fatal("references should be the same")
+	}
+
+	if !reflect.DeepEqual(&repo.referrersPingLock, &crepo.referrersPingLock) {
+		t.Fatal("referrersPingLock should be different")
+	}
+
+	if !reflect.DeepEqual(&repo.referrersMergePool, &crepo.referrersMergePool) {
+		t.Fatal("referrersMergePool should be different")
+	}
+}