Skip to content

Commit

Permalink
fix: sharing not thread safe go-git tree and storer
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexey-igrychev
alexey-igrychev committed Sep 22, 2021
1 parent 50b9a0c commit 1e2755b
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 96 deletions.
4 changes: 2 additions & 2 deletions pkg/build/stage/git_mapping.go
View file
Expand Up @@ -926,12 +926,12 @@ func (gm *GitMapping) getArchiveType(ctx context.Context, commit string) (git_re
func (gm *GitMapping) isEmpty(ctx context.Context, c Conveyor) (bool, error) {
commitInfo, err := gm.GetLatestCommitInfo(ctx, c)
if err != nil {
return true, err
return true, fmt.Errorf("unable to get latest commit info: %s", err)
}

pathScope, err := gm.getPathScope(ctx, commitInfo.Commit)
if err != nil {
return true, err
return true, fmt.Errorf("unable to get path scope: %s", err)
}

isAnyMatchesByGitAdd, err := gm.GitRepo().IsAnyCommitTreeEntriesMatched(ctx, commitInfo.Commit, pathScope, gm.getPathMatcher(), true)
Expand Down
47 changes: 42 additions & 5 deletions pkg/git_repo/repo_handle/handle.go
View file
Expand Up @@ -2,30 +2,67 @@ package repo_handle

import (
"fmt"
"io/ioutil"
"sync"

"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/config"
"github.com/go-git/go-git/v5/plumbing"
)

type handle struct {
repository *git.Repository
submoduleHandleList []SubmoduleHandle

mutex sync.Mutex
mutex *sync.Mutex
}

func newHandle(repository *git.Repository) *handle {
func newHandle(repository *git.Repository, mutex *sync.Mutex) *handle {
return &handle{
repository: repository,
mutex: mutex,
}
}

func (h *handle) WithRepository(f func(r Repository) error) error {
func (h *handle) ReadBlobObjectContent(hash plumbing.Hash) ([]byte, error) {
h.mutex.Lock()
defer h.mutex.Unlock()

return f(h.repository)
obj, err := h.repository.BlobObject(hash)
if err != nil {
return nil, fmt.Errorf("unable to get blob %q object: %s", hash, err)
}

f, err := obj.Reader()
if err != nil {
return nil, err
}
defer f.Close()

data, err := ioutil.ReadAll(f)
if err != nil {
return nil, fmt.Errorf("unable to read blob %q content: %s", hash, err)
}

return data, nil
}

func (h *handle) GetCommitTree(hash plumbing.Hash) (TreeHandle, error) {
h.mutex.Lock()
defer h.mutex.Unlock()

commit, err := h.repository.CommitObject(hash)
if err != nil {
return nil, fmt.Errorf("unable to get commit %q object: %s", hash, err)
}

tree, err := commit.Tree()
if err != nil {
return nil, fmt.Errorf("unable to get commit %q tree: %s", hash, err)
}

treeHandle := newTreeHandle(tree, h.mutex)
return treeHandle, nil
}

func (h *handle) Submodule(submodulePath string) (SubmoduleHandle, error) {
Expand All @@ -48,7 +85,7 @@ type submoduleHandle struct {
status *git.SubmoduleStatus
}

func newRepositorySubmoduleHandle(handle Handle, config *config.Submodule, status *git.SubmoduleStatus) *submoduleHandle {
func newSubmoduleHandle(handle Handle, config *config.Submodule, status *git.SubmoduleStatus) *submoduleHandle {
return &submoduleHandle{
Handle: handle,
config: config,
Expand Down
28 changes: 18 additions & 10 deletions pkg/git_repo/repo_handle/interface.go
View file
Expand Up @@ -2,6 +2,7 @@ package repo_handle

import (
"fmt"
"sync"

"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/config"
Expand All @@ -13,9 +14,17 @@ import (
// caching the necessary data from the worktree during initialization,
// and then working exclusively with git objects.
type Handle interface {
WithRepository(func(r Repository) error) error
Submodule(submodulePath string) (SubmoduleHandle, error)
Submodules() []SubmoduleHandle
ReadBlobObjectContent(hash plumbing.Hash) ([]byte, error)
GetCommitTree(hash plumbing.Hash) (TreeHandle, error)
}

type TreeHandle interface {
Hash() plumbing.Hash
Entries() []object.TreeEntry
Tree(path string) (TreeHandle, error)
FindEntry(path string) (*object.TreeEntry, error)
}

type SubmoduleHandle interface {
Expand All @@ -24,15 +33,14 @@ type SubmoduleHandle interface {
Status() *git.SubmoduleStatus
}

type Repository interface {
CommitObject(h plumbing.Hash) (*object.Commit, error)
BlobObject(h plumbing.Hash) (*object.Blob, error)
func NewHandle(repository *git.Repository) (Handle, error) {
return newHandleWithSubmodules(repository, &sync.Mutex{})
}

func NewHandle(repository *git.Repository) (Handle, error) {
h := newHandle(repository)
func newHandleWithSubmodules(repository *git.Repository, mutex *sync.Mutex) (Handle, error) {
h := newHandle(repository, mutex)

submoduleHandleList, err := getSubmoduleHandleList(repository)
submoduleHandleList, err := getSubmoduleHandleList(repository, mutex)
if err != nil {
return nil, err
}
Expand All @@ -42,7 +50,7 @@ func NewHandle(repository *git.Repository) (Handle, error) {
return h, nil
}

func getSubmoduleHandleList(parentRepository *git.Repository) ([]SubmoduleHandle, error) {
func getSubmoduleHandleList(parentRepository *git.Repository, mutex *sync.Mutex) ([]SubmoduleHandle, error) {
var list []SubmoduleHandle

w, err := parentRepository.Worktree()
Expand All @@ -66,12 +74,12 @@ func getSubmoduleHandleList(parentRepository *git.Repository) ([]SubmoduleHandle
return nil, fmt.Errorf("unable to get submodule %q status: %s", s.Config().Path, err)
}

handle, err := NewHandle(submoduleRepository)
handle, err := newHandleWithSubmodules(submoduleRepository, mutex)
if err != nil {
return nil, err
}

submoduleHandle := newRepositorySubmoduleHandle(handle, s.Config(), submoduleStatus)
submoduleHandle := newSubmoduleHandle(handle, s.Config(), submoduleStatus)
list = append(list, submoduleHandle)
}

Expand Down
49 changes: 49 additions & 0 deletions pkg/git_repo/repo_handle/tree_handle.go
View file
@@ -0,0 +1,49 @@
package repo_handle

import (
"fmt"
"sync"

"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/object"
)

type treeHandle struct {
tree *object.Tree
mutex *sync.Mutex
}

func newTreeHandle(tree *object.Tree, mutex *sync.Mutex) *treeHandle {
return &treeHandle{tree: tree, mutex: mutex}
}

func (h *treeHandle) Hash() plumbing.Hash {
return h.tree.Hash
}

func (h *treeHandle) Tree(path string) (TreeHandle, error) {
h.mutex.Lock()
defer h.mutex.Unlock()

tree, err := h.tree.Tree(path)
if err != nil {
return nil, fmt.Errorf("unable to get tree %q: %s", path, err)
}

treeHandle := newTreeHandle(tree, h.mutex)
return treeHandle, nil
}

func (h *treeHandle) FindEntry(path string) (*object.TreeEntry, error) {
h.mutex.Lock()
defer h.mutex.Unlock()

return h.tree.FindEntry(path)
}

func (h *treeHandle) Entries() []object.TreeEntry {
h.mutex.Lock()
defer h.mutex.Unlock()

return h.tree.Entries
}
67 changes: 14 additions & 53 deletions pkg/true_git/ls_tree/ls_tree.go
View file
Expand Up @@ -80,7 +80,7 @@ func LsTree(ctx context.Context, repoHandle repo_handle.Handle, commit string, o
func lsTree(ctx context.Context, repoHandle repo_handle.Handle, commit string, opts LsTreeOptions) (*Result, error) {
res := NewResult(commit, "", []*LsTreeEntry{}, []*SubmoduleResult{})

tree, err := getCommitTree(repoHandle, commit)
tree, err := repoHandle.GetCommitTree(plumbing.NewHash(commit))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -112,7 +112,7 @@ func lsTree(ctx context.Context, repoHandle repo_handle.Handle, commit string, o
TreeEntry: object.TreeEntry{
Name: "",
Mode: filemode.Dir,
Hash: tree.Hash,
Hash: tree.Hash(),
},
}

Expand Down Expand Up @@ -151,33 +151,7 @@ func lsTree(ctx context.Context, repoHandle repo_handle.Handle, commit string, o
return res, nil
}

func getCommitTree(repoHandle repo_handle.Handle, commit string) (*object.Tree, error) {
commitHash, err := newHash(commit)
if err != nil {
return nil, fmt.Errorf("invalid commit %q: %s", commit, err)
}

var tree *object.Tree
if err := repoHandle.WithRepository(func(repository repo_handle.Repository) error {
commitObj, err := repository.CommitObject(commitHash)
if err != nil {
return fmt.Errorf("unable to get %s commit info: %s", commit, err)
}

tree, err = commitObj.Tree()
if err != nil {
return err
}

return nil
}); err != nil {
return nil, err
}

return tree, nil
}

func processSpecificEntryFilepath(ctx context.Context, repoHandle repo_handle.Handle, tree *object.Tree, repositoryFullFilepath, treeFullFilepath, treeEntryFilepath string, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
func processSpecificEntryFilepath(ctx context.Context, repoHandle repo_handle.Handle, tree repo_handle.TreeHandle, repositoryFullFilepath, treeFullFilepath, treeEntryFilepath string, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
for _, submoduleHandle := range repoHandle.Submodules() {
submoduleEntryFilepath := filepath.FromSlash(submoduleHandle.Config().Path)
submoduleFullFilepath := filepath.Join(treeFullFilepath, submoduleEntryFilepath)
Expand Down Expand Up @@ -227,8 +201,8 @@ func processSpecificEntryFilepath(ctx context.Context, repoHandle repo_handle.Ha
return lsTreeEntries, submodulesLsTreeEntries, nil
}

func lsTreeWalk(ctx context.Context, repoHandle repo_handle.Handle, tree *object.Tree, repositoryFullFilepath, treeFullFilepath string, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
for _, treeEntry := range tree.Entries {
func lsTreeWalk(ctx context.Context, repoHandle repo_handle.Handle, tree repo_handle.TreeHandle, repositoryFullFilepath, treeFullFilepath string, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
for _, treeEntry := range tree.Entries() {
lsTreeEntry := &LsTreeEntry{
FullFilepath: filepath.Join(treeFullFilepath, treeEntry.Name),
TreeEntry: treeEntry,
Expand All @@ -246,7 +220,7 @@ func lsTreeWalk(ctx context.Context, repoHandle repo_handle.Handle, tree *object
return
}

func lsTreeEntryMatch(ctx context.Context, repoHandle repo_handle.Handle, tree *object.Tree, repositoryFullFilepath, treeFullFilepath string, lsTreeEntry *LsTreeEntry, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
func lsTreeEntryMatch(ctx context.Context, repoHandle repo_handle.Handle, tree repo_handle.TreeHandle, repositoryFullFilepath, treeFullFilepath string, lsTreeEntry *LsTreeEntry, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
switch lsTreeEntry.Mode {
case filemode.Dir:
return lsTreeDirEntryMatch(ctx, repoHandle, tree, repositoryFullFilepath, treeFullFilepath, lsTreeEntry, opts)
Expand All @@ -257,7 +231,7 @@ func lsTreeEntryMatch(ctx context.Context, repoHandle repo_handle.Handle, tree *
}
}

func lsTreeDirEntryMatch(ctx context.Context, repoHandle repo_handle.Handle, tree *object.Tree, repositoryFullFilepath, treeFullFilepath string, lsTreeEntry *LsTreeEntry, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
func lsTreeDirEntryMatch(ctx context.Context, repoHandle repo_handle.Handle, tree repo_handle.TreeHandle, repositoryFullFilepath, treeFullFilepath string, lsTreeEntry *LsTreeEntry, opts LsTreeOptions) (lsTreeEntries []*LsTreeEntry, submodulesResults []*SubmoduleResult, err error) {
if err := lsTreeDirOrSubmoduleEntryMatchBase(
lsTreeEntry.FullFilepath,
opts,
Expand Down Expand Up @@ -397,7 +371,7 @@ func lsTreeFileEntryMatch(ctx context.Context, lsTreeEntry *LsTreeEntry, opts Ls
return
}

func treeFindEntry(_ context.Context, tree *object.Tree, treeFullFilepath, treeEntryFilepath string) (*LsTreeEntry, error) {
func treeFindEntry(_ context.Context, tree repo_handle.TreeHandle, treeFullFilepath, treeEntryFilepath string) (*LsTreeEntry, error) {
formattedTreeEntryPath := filepath.ToSlash(treeEntryFilepath)
treeEntry, err := tree.FindEntry(formattedTreeEntryPath)
if err != nil {
Expand All @@ -410,7 +384,7 @@ func treeFindEntry(_ context.Context, tree *object.Tree, treeFullFilepath, treeE
}, nil
}

func treeTree(tree *object.Tree, treeFullFilepath, treeDirEntryFullFilepath string) (*object.Tree, error) {
func treeTree(tree repo_handle.TreeHandle, treeFullFilepath, treeDirEntryFullFilepath string) (repo_handle.TreeHandle, error) {
treeDirEntryFilepath, err := filepath.Rel(treeFullFilepath, treeDirEntryFullFilepath)
if err != nil || treeDirEntryFilepath == "." || treeDirEntryFilepath == ".." || strings.HasPrefix(treeDirEntryFilepath, ".."+string(os.PathSeparator)) {
panic(fmt.Sprintf("unexpected paths: %s, %s", treeFullFilepath, treeDirEntryFullFilepath))
Expand All @@ -425,28 +399,15 @@ func treeTree(tree *object.Tree, treeFullFilepath, treeDirEntryFullFilepath stri
return entryTree, nil
}

func getSubmoduleTree(repoHandle repo_handle.SubmoduleHandle) (*object.Tree, error) {
func getSubmoduleTree(repoHandle repo_handle.SubmoduleHandle) (repo_handle.TreeHandle, error) {
submoduleName := repoHandle.Config().Name
expectedCommit := repoHandle.Status().Expected

var submoduleTree *object.Tree
if err := repoHandle.WithRepository(func(submoduleRepository repo_handle.Repository) error {
commit, err := submoduleRepository.CommitObject(expectedCommit)
if err != nil {
return fmt.Errorf("cannot inspect submodule %q commit %q: %s", submoduleName, expectedCommit, err)
}

submoduleTree, err = commit.Tree()
if err != nil {
return fmt.Errorf("cannot get submodule %q commit %q tree: %s", submoduleName, expectedCommit, err)
}

return nil
}); err != nil {
return nil, err
tree, err := repoHandle.GetCommitTree(expectedCommit)
if err != nil {
return nil, fmt.Errorf("unable to get submodule %q commit %q tree: %s", submoduleName, expectedCommit, err)
}

return submoduleTree, nil
return tree, nil
}

func debug() bool {
Expand Down
28 changes: 2 additions & 26 deletions pkg/true_git/ls_tree/result.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"crypto/sha256"
"fmt"
"io/ioutil"
"path/filepath"
"sort"

Expand Down Expand Up @@ -103,7 +102,7 @@ func (r *Result) LsTree(ctx context.Context, repoHandle repo_handle.Handle, opts
func (r *Result) lsTree(ctx context.Context, repoHandle repo_handle.Handle, opts LsTreeOptions) (*Result, error) {
res := NewResult(r.commit, r.repositoryFullFilepath, []*LsTreeEntry{}, []*SubmoduleResult{})

tree, err := getCommitTree(repoHandle, r.commit)
tree, err := repoHandle.GetCommitTree(plumbing.NewHash(r.commit))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -330,28 +329,5 @@ func (r *Result) LsTreeEntryContent(mainRepoHandle repo_handle.Handle, relPath s
panic("unexpected condition")
}

var data []byte
if err := entryRepoHandle.WithRepository(func(entryRepository repo_handle.Repository) error {
obj, err := entryRepository.BlobObject(entry.Hash)
if err != nil {
return fmt.Errorf("unable to get tree entry %q blob object: %s", entry.FullFilepath, err)
}

f, err := obj.Reader()
if err != nil {
return err
}
defer f.Close()

data, err = ioutil.ReadAll(f)
if err != nil {
return fmt.Errorf("unable to read tree entry %q content: %s", relPath, err)
}

return nil
}); err != nil {
return nil, err
}

return data, nil
return entryRepoHandle.ReadBlobObjectContent(entry.Hash)
}

0 comments on commit 1e2755b

Please sign in to comment.