From f81d358a319eae9bcee2332809c8e15a5976d501 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Tue, 18 Aug 2020 11:09:44 -0400 Subject: [PATCH] API: wrap access to go-git in mutex locks Apparently go-git is not thread-safe. I yearn for the day when threads are a forgotten relic of a barbaric time. --- api/graph/model/blob.go | 7 +++--- api/graph/model/commit.go | 5 ++--- api/graph/model/object.go | 5 +++-- api/graph/model/reference.go | 7 ++++-- api/graph/model/repository.go | 10 +++++---- api/graph/model/repowrapper.go | 16 +++++++++++++ api/graph/model/tree.go | 7 +++--- api/graph/schema.resolvers.go | 41 +++++++++++++++++++++++++--------- 8 files changed, 68 insertions(+), 30 deletions(-) create mode 100644 api/graph/model/repowrapper.go diff --git a/api/graph/model/blob.go b/api/graph/model/blob.go index 7fd1da0..fd70253 100644 --- a/api/graph/model/blob.go +++ b/api/graph/model/blob.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "unicode/utf8" - "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing/object" ) @@ -18,7 +17,7 @@ type BinaryBlob struct { Base64 string `json:"base64"` blob *object.Blob - repo *git.Repository + repo *RepoWrapper } func (BinaryBlob) IsObject() {} @@ -33,13 +32,13 @@ type TextBlob struct { Text string `json:"text"` blob *object.Blob - repo *git.Repository + repo *RepoWrapper } func (TextBlob) IsObject() {} func (TextBlob) IsBlob() {} -func BlobFromObject(repo *git.Repository, obj *object.Blob) Object { +func BlobFromObject(repo *RepoWrapper, obj *object.Blob) Object { reader, err := obj.Reader() if err != nil { panic(err) diff --git a/api/graph/model/commit.go b/api/graph/model/commit.go index d89168e..7381f76 100644 --- a/api/graph/model/commit.go +++ b/api/graph/model/commit.go @@ -3,7 +3,6 @@ package model import ( "context" - "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing/object" ) @@ -14,7 +13,7 @@ type Commit struct { Raw string `json:"raw"` commit *object.Commit - repo *git.Repository + repo *RepoWrapper } func (Commit) IsObject() {} @@ -70,7 +69,7 @@ func (c *Commit) Parents() []*Commit { return parents } -func CommitFromObject(repo *git.Repository, obj *object.Commit) *Commit { +func CommitFromObject(repo *RepoWrapper, obj *object.Commit) *Commit { return &Commit{ Type: ObjectTypeCommit, ID: obj.ID().String(), diff --git a/api/graph/model/object.go b/api/graph/model/object.go index 386ec83..c611b48 100644 --- a/api/graph/model/object.go +++ b/api/graph/model/object.go @@ -3,7 +3,6 @@ package model import ( "fmt" - "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" ) @@ -12,8 +11,10 @@ type Object interface { IsObject() } -func LookupObject(repo *git.Repository, hash plumbing.Hash) (Object, error) { +func LookupObject(repo *RepoWrapper, hash plumbing.Hash) (Object, error) { + repo.Lock() obj, err := repo.Object(plumbing.AnyObject, hash) + repo.Unlock() if err != nil { return nil, fmt.Errorf("lookup object %s: %w", hash.String(), err) } diff --git a/api/graph/model/reference.go b/api/graph/model/reference.go index afc3ab2..ac342b6 100644 --- a/api/graph/model/reference.go +++ b/api/graph/model/reference.go @@ -10,11 +10,14 @@ type Reference struct { } func (r *Reference) Follow() Object { - ref, err := r.Repo.Repo().Reference(r.Ref.Name(), true) + repo := r.Repo.Repo() + repo.Lock() + ref, err := repo.Reference(r.Ref.Name(), true) + repo.Unlock() if err != nil { panic(err) } - obj, err := LookupObject(r.Repo.Repo(), ref.Hash()) + obj, err := LookupObject(repo, ref.Hash()) if err != nil { println(err) panic(err) diff --git a/api/graph/model/repository.go b/api/graph/model/repository.go index b4048c0..8b64d55 100644 --- a/api/graph/model/repository.go +++ b/api/graph/model/repository.go @@ -27,23 +27,25 @@ type Repository struct { OwnerID int alias string - repo *git.Repository + repo *RepoWrapper } -func (r *Repository) Repo() *git.Repository { +func (r *Repository) Repo() *RepoWrapper { if r.repo != nil { return r.repo } - var err error - r.repo, err = git.PlainOpen(r.Path) + repo, err := git.PlainOpen(r.Path) if err != nil { panic(err) } + r.repo = WrapRepo(repo) return r.repo } func (r *Repository) Head() *Reference { + r.Repo().Lock() ref, err := r.Repo().Head() + r.repo.Unlock() if err != nil { if err == plumbing.ErrReferenceNotFound { return nil diff --git a/api/graph/model/repowrapper.go b/api/graph/model/repowrapper.go new file mode 100644 index 0000000..a5ed809 --- /dev/null +++ b/api/graph/model/repowrapper.go @@ -0,0 +1,16 @@ +package model + +import ( + "sync" + + "github.com/go-git/go-git/v5" +) + +type RepoWrapper struct { + *git.Repository + sync.Mutex +} + +func WrapRepo(repo *git.Repository) *RepoWrapper { + return &RepoWrapper{repo, sync.Mutex{}} +} diff --git a/api/graph/model/tree.go b/api/graph/model/tree.go index 32cb931..e6cd3ec 100644 --- a/api/graph/model/tree.go +++ b/api/graph/model/tree.go @@ -3,7 +3,6 @@ package model import ( "sort" - "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" ) @@ -15,7 +14,7 @@ type Tree struct { Raw string `json:"raw"` tree *object.Tree - repo *git.Repository + repo *RepoWrapper } func (Tree) IsObject() {} @@ -25,7 +24,7 @@ type TreeEntry struct { Mode int `json:"mode"` hash plumbing.Hash - repo *git.Repository + repo *RepoWrapper } func (ent *TreeEntry) ID() string { @@ -72,7 +71,7 @@ func (tree *Tree) GetEntries() []*TreeEntry { return qlents } -func TreeFromObject(repo *git.Repository, obj *object.Tree) *Tree { +func TreeFromObject(repo *RepoWrapper, obj *object.Tree) *Tree { return &Tree{ Type: ObjectTypeTree, ID: obj.ID().String(), diff --git a/api/graph/schema.resolvers.go b/api/graph/schema.resolvers.go index 5247872..bc89c32 100644 --- a/api/graph/schema.resolvers.go +++ b/api/graph/schema.resolvers.go @@ -172,11 +172,14 @@ func (r *referenceResolver) Artifacts(ctx context.Context, obj *model.Reference, cursor = gqlmodel.NewCursor(nil) } - ref, err := obj.Repo.Repo().Reference(obj.Ref.Name(), true) + repo := obj.Repo.Repo() + repo.Lock() + defer repo.Unlock() + ref, err := repo.Reference(obj.Ref.Name(), true) if err != nil { return nil, err } - o, err := obj.Repo.Repo().Object(plumbing.TagObject, ref.Hash()) + o, err := repo.Object(plumbing.TagObject, ref.Hash()) if err == plumbing.ErrObjectNotFound { return &model.ArtifactCursor{nil, cursor}, nil } else if err != nil { @@ -233,7 +236,10 @@ func (r *repositoryResolver) Objects(ctx context.Context, obj *model.Repository, } func (r *repositoryResolver) References(ctx context.Context, obj *model.Repository, cursor *gqlmodel.Cursor) (*model.ReferenceCursor, error) { - iter, err := obj.Repo().References() + repo := obj.Repo() + repo.Lock() + defer repo.Unlock() + iter, err := repo.References() if err != nil { return nil, err } @@ -286,11 +292,14 @@ func (r *repositoryResolver) Log(ctx context.Context, obj *model.Repository, cur } } + repo := obj.Repo() opts := &git.LogOptions{ Order: git.LogOrderCommitterTime, } if cursor.Next != "" { - rev, err := obj.Repo().ResolveRevision(plumbing.Revision(cursor.Next)) + repo.Lock() + rev, err := repo.ResolveRevision(plumbing.Revision(cursor.Next)) + repo.Unlock() if err != nil { return nil, err } @@ -300,14 +309,16 @@ func (r *repositoryResolver) Log(ctx context.Context, obj *model.Repository, cur opts.From = *rev } - log, err := obj.Repo().Log(opts) + repo.Lock() + log, err := repo.Log(opts) + repo.Unlock() if err != nil { return nil, err } var commits []*model.Commit log.ForEach(func(c *object.Commit) error { - commits = append(commits, model.CommitFromObject(obj.Repo(), c)) + commits = append(commits, model.CommitFromObject(repo, c)) if len(commits) == cursor.Count+1 { return storer.ErrStop } @@ -333,14 +344,19 @@ func (r *repositoryResolver) Path(ctx context.Context, obj *model.Repository, re if revspec != nil { rev = plumbing.Revision(*revspec) } - hash, err := obj.Repo().ResolveRevision(rev) + repo := obj.Repo() + repo.Lock() + hash, err := repo.ResolveRevision(rev) + repo.Unlock() if err != nil { return nil, err } if hash == nil { return nil, fmt.Errorf("No such object") } - o, err := obj.Repo().Object(plumbing.CommitObject, *hash) + repo.Lock() + o, err := repo.Object(plumbing.CommitObject, *hash) + repo.Unlock() if err != nil { return nil, err } @@ -352,21 +368,24 @@ func (r *repositoryResolver) Path(ctx context.Context, obj *model.Repository, re if treeObj, err := commit.Tree(); err != nil { panic(err) } else { - tree = model.TreeFromObject(obj.Repo(), treeObj) + tree = model.TreeFromObject(repo, treeObj) } return tree.Entry(path), nil } func (r *repositoryResolver) RevparseSingle(ctx context.Context, obj *model.Repository, revspec string) (*model.Commit, error) { rev := plumbing.Revision(revspec) - hash, err := obj.Repo().ResolveRevision(rev) + repo := obj.Repo() + repo.Lock() + hash, err := repo.ResolveRevision(rev) + repo.Unlock() if err != nil { return nil, err } if hash == nil { return nil, fmt.Errorf("No such object") } - o, err := model.LookupObject(obj.Repo(), *hash) + o, err := model.LookupObject(repo, *hash) if err != nil { return nil, err } -- 2.38.4