fix race condition on ObjectLRU #544
Conversation
Codecov Report
@@ Coverage Diff @@
## master #544 +/- ##
==========================================
- Coverage 78.07% 77.48% -0.59%
==========================================
Files 129 129
Lines 9792 9798 +6
==========================================
- Hits 7645 7592 -53
- Misses 1316 1388 +72
+ Partials 831 818 -13
Continue to review full report at Codecov.
|
plumbing/cache/object_test.go
Outdated
| for i := 0; i < 1000; i++ { | ||
| wg.Add(1) | ||
| go func(i int) { | ||
| s.c.Put(newObject(fmt.Sprint(i), FileSize(i))) |
There was a problem hiding this comment.
Do we also want to exercise the other operations that require synchronization? Like Get and Clear?
Signed-off-by: Miguel Molina <[email protected]>
erizocosmico
force-pushed
the
fix/race-condition-object-lru
branch
from
ef35f91 to
3fd3988
Compare
|
@orirawlings amended with usage of |
| @@ -67,6 +72,9 @@ func (c *ObjectLRU) Put(obj plumbing.EncodedObject) { | |||
| // Get returns an object by its hash. It marks the object as used. If the object | |||
| // is not in the cache, (nil, false) will be returned. | |||
| func (c *ObjectLRU) Get(k plumbing.Hash) (plumbing.EncodedObject, bool) { | |||
| c.mut.Lock() | |||
There was a problem hiding this comment.
Maybe we can use a RWLock? as suggested at: https://blog.golang.org/go-maps-in-action
There was a problem hiding this comment.
Get mutates the state in c.ll.MoveToFront(ee), that's why I used a Mutex instead of a RWMutex
With concurrent usage, it could lead to a
concurrent map accesspanic on runtime.Sadly, the test only adds value with
-raceenabled. If you have any idea how to get that panic on the test, it would improve its robustness.