Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

Commit

Permalink
storage/filesystem: check file object before using cache
Browse files Browse the repository at this point in the history
If the cache is shared between several repositories getFromUnpacked can
erroneously return an object from other repository.

This decreases performance a little bit as there's an extra fs operation
when the object is in the cache but is correct when the cache is shared.

Signed-off-by: Javi Fontan <jfontan@gmail.com>
  • Loading branch information
jfontan committed Jan 30, 2019
1 parent 434611b commit 34cd3cb
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
5 changes: 0 additions & 5 deletions storage/filesystem/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,6 @@ func (s *ObjectStorage) DeltaObject(t plumbing.ObjectType,
}

func (s *ObjectStorage) getFromUnpacked(h plumbing.Hash) (obj plumbing.EncodedObject, err error) {
if cacheObj, found := s.objectCache.Get(h); found {
return cacheObj, nil
}

f, err := s.dir.Object(h)
if err != nil {
if os.IsNotExist(err) {
Expand All @@ -319,7 +315,6 @@ func (s *ObjectStorage) getFromUnpacked(h plumbing.Hash) (obj plumbing.EncodedOb

return nil, err
}

defer ioutil.CheckClose(f, &err)

obj = s.NewEncodedObject()
Expand Down
17 changes: 17 additions & 0 deletions storage/filesystem/object_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,23 @@ func (s *FsSuite) TestPackfileIterKeepDescriptors(c *C) {
})
}

func (s *FsSuite) TestGetFromObjectFileSharedCache(c *C) {
f1 := fixtures.ByTag("worktree").One().DotGit()
f2 := fixtures.ByTag("worktree").ByTag("submodule").One().DotGit()

ch := cache.NewObjectLRUDefault()
o1 := NewObjectStorage(dotgit.New(f1), ch)
o2 := NewObjectStorage(dotgit.New(f2), ch)

expected := plumbing.NewHash("af2d6a6954d532f8ffb47615169c8fdf9d383a1a")
obj, err := o1.EncodedObject(plumbing.CommitObject, expected)
c.Assert(err, IsNil)
c.Assert(obj.Hash(), Equals, expected)

obj, err = o2.EncodedObject(plumbing.CommitObject, expected)
c.Assert(err, Equals, plumbing.ErrObjectNotFound)
}

func BenchmarkPackfileIter(b *testing.B) {
if err := fixtures.Init(); err != nil {
b.Fatal(err)
Expand Down

0 comments on commit 34cd3cb

Please sign in to comment.