Skip to content

Commit

Permalink
fix: Avoid a copy of sync.Mutex in Repository (#603)
Browse files Browse the repository at this point in the history
Closes #600
Signed-off-by: Kyle M. Tarplee <kmtarplee@ieee.org>
  • Loading branch information
ktarplee authored and shizhMSFT committed Oct 20, 2023
1 parent bfef8c5 commit eb31910
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 12 deletions.
32 changes: 20 additions & 12 deletions registry/remote/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
}

Expand Down
21 changes: 21 additions & 0 deletions registry/remote/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

0 comments on commit eb31910

Please sign in to comment.