Skip to content

Commit e5cabfa

Browse files
authored
fix: Avoid a copy of sync.Mutex in Repository (#603)
Closes #600 Signed-off-by: Kyle M. Tarplee <kmtarplee@ieee.org>
1 parent cb8c8bc commit e5cabfa

File tree

2 files changed

+41
-12
lines changed

2 files changed

+41
-12
lines changed

registry/remote/repository.go

+20-12
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ type Repository struct {
147147
// - https://www.rfc-editor.org/rfc/rfc7234#section-5.5
148148
HandleWarning func(warning Warning)
149149

150-
// NOTE: Must keep fields in sync with newRepositoryWithOptions function.
150+
// NOTE: Must keep fields in sync with clone().
151151

152152
// referrersState represents that if the repository supports Referrers API.
153153
// default: referrersStateUnknown
@@ -186,16 +186,24 @@ func newRepositoryWithOptions(ref registry.Reference, opts *RepositoryOptions) (
186186
if err := ref.ValidateRepository(); err != nil {
187187
return nil, err
188188
}
189+
repo := (*Repository)(opts).clone()
190+
repo.Reference = ref
191+
return repo, nil
192+
}
193+
194+
// clone makes a copy of the Repository being careful not to copy non-copyable fields (sync.Mutex and syncutil.Pool types)
195+
func (r *Repository) clone() *Repository {
189196
return &Repository{
190-
Client: opts.Client,
191-
Reference: ref,
192-
PlainHTTP: opts.PlainHTTP,
193-
SkipReferrersGC: opts.SkipReferrersGC,
194-
ManifestMediaTypes: slices.Clone(opts.ManifestMediaTypes),
195-
TagListPageSize: opts.TagListPageSize,
196-
ReferrerListPageSize: opts.ReferrerListPageSize,
197-
MaxMetadataBytes: opts.MaxMetadataBytes,
198-
}, nil
197+
Client: r.Client,
198+
Reference: r.Reference,
199+
PlainHTTP: r.PlainHTTP,
200+
ManifestMediaTypes: slices.Clone(r.ManifestMediaTypes),
201+
TagListPageSize: r.TagListPageSize,
202+
ReferrerListPageSize: r.ReferrerListPageSize,
203+
MaxMetadataBytes: r.MaxMetadataBytes,
204+
SkipReferrersGC: r.SkipReferrersGC,
205+
HandleWarning: r.HandleWarning,
206+
}
199207
}
200208

201209
// SetReferrersCapability indicates the Referrers API capability of the remote
@@ -803,10 +811,10 @@ func (s *blobStore) Mount(ctx context.Context, desc ocispec.Descriptor, fromRepo
803811
// sibling returns a blob store for another repository in the same
804812
// registry.
805813
func (s *blobStore) sibling(otherRepoName string) *blobStore {
806-
otherRepo := *s.repo
814+
otherRepo := s.repo.clone()
807815
otherRepo.Reference.Repository = otherRepoName
808816
return &blobStore{
809-
repo: &otherRepo,
817+
repo: otherRepo,
810818
}
811819
}
812820

registry/remote/repository_test.go

+21
Original file line numberDiff line numberDiff line change
@@ -7447,3 +7447,24 @@ func TestRepository_do(t *testing.T) {
74477447
t.Errorf("Repository.do() = %v, want %v", gotWarnings, wantWarnings)
74487448
}
74497449
}
7450+
7451+
func TestRepository_clone(t *testing.T) {
7452+
repo, err := NewRepository("localhost:1234/repo/image")
7453+
if err != nil {
7454+
t.Fatalf("invalid repository: %v", err)
7455+
}
7456+
7457+
crepo := repo.clone()
7458+
7459+
if repo.Reference != crepo.Reference {
7460+
t.Fatal("references should be the same")
7461+
}
7462+
7463+
if !reflect.DeepEqual(&repo.referrersPingLock, &crepo.referrersPingLock) {
7464+
t.Fatal("referrersPingLock should be different")
7465+
}
7466+
7467+
if !reflect.DeepEqual(&repo.referrersMergePool, &crepo.referrersMergePool) {
7468+
t.Fatal("referrersMergePool should be different")
7469+
}
7470+
}

0 commit comments

Comments
 (0)