Skip to content

Commit 07a2e1c

Browse files
committed
add mutex and make sure Entries() doesn't return original map
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
1 parent 7893237 commit 07a2e1c

3 files changed

Lines changed: 22 additions & 13 deletions

File tree

kai_analyzer_rpc/pkg/service/analyzer.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ type analyzer struct {
6161
discoveryCacheMutex sync.Mutex
6262
discoveryCache []konveyor.RuleSet
6363
cache IncidentsCache
64-
cacheMutex sync.RWMutex
6564

6665
contextLines int
6766
location string
@@ -176,7 +175,6 @@ func NewAnalyzer(limitIncidents, limitCodeSnips, contextLines int, location, inc
176175
discoveryCache: []konveyor.RuleSet{},
177176
discoveryCacheMutex: sync.Mutex{},
178177
cache: NewIncidentsCache(log),
179-
cacheMutex: sync.RWMutex{},
180178
}, nil
181179

182180
}
@@ -328,16 +326,11 @@ func (a *analyzer) NotifyFileChanges(client *rpc.Client, args NotifyFileChangesA
328326
}
329327

330328
func (a *analyzer) setCache(rulesets []konveyor.RuleSet) {
331-
a.cacheMutex.Lock()
332-
defer a.cacheMutex.Unlock()
333329
a.cache = NewIncidentsCache(a.Logger)
334-
335330
a.addRulesetsToCache(rulesets)
336331
}
337332

338333
func (a *analyzer) updateCache(rulesets []konveyor.RuleSet, includedPaths []string) {
339-
a.cacheMutex.Lock()
340-
defer a.cacheMutex.Unlock()
341334
if includedPaths != nil {
342335
a.invalidateCachePerFile(includedPaths)
343336
}
@@ -379,9 +372,6 @@ func (a *analyzer) invalidateCachePerFile(paths []string) {
379372
}
380373

381374
func (a *analyzer) createRulesetsFromCache() []konveyor.RuleSet {
382-
a.cacheMutex.RLock()
383-
defer a.cacheMutex.RUnlock()
384-
385375
ruleSetMap := map[string]konveyor.RuleSet{}
386376
a.Logger.V(8).Info("cache", "cacheVal", a.cache)
387377
for filePath, cacheValue := range a.cache.Entries() {

kai_analyzer_rpc/pkg/service/cache.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package service
33
import (
44
"path/filepath"
55
"strings"
6+
"sync"
67

78
"github.com/go-logr/logr"
89
"github.com/konveyor/analyzer-lsp/output/v1/konveyor"
@@ -27,26 +28,32 @@ func NewIncidentsCache(logger logr.Logger) IncidentsCache {
2728
return &incidentsCache{
2829
cache: map[string][]CacheValue{},
2930
logger: logger,
31+
mutex: sync.RWMutex{},
3032
}
3133
}
3234

3335
type incidentsCache struct {
3436
cache map[string][]CacheValue
3537
logger logr.Logger
38+
mutex sync.RWMutex
3639
}
3740

3841
func (i *incidentsCache) Len() int {
3942
return len(i.cache)
4043
}
4144

4245
func (i *incidentsCache) Get(path string) ([]CacheValue, bool) {
46+
i.mutex.RLock()
47+
defer i.mutex.RUnlock()
4348
normalizedPath := normalizePath(path)
4449
i.logger.V(8).Info("getting cache entry for path", "path", path, "normalizedPath", normalizedPath)
4550
val, ok := i.cache[normalizedPath]
4651
return val, ok
4752
}
4853

4954
func (i *incidentsCache) Add(path string, value CacheValue) {
55+
i.mutex.Lock()
56+
defer i.mutex.Unlock()
5057
normalizedPath := normalizePath(path)
5158
i.logger.V(8).Info("adding cache entry for path", "path", path, "normalizedPath", normalizedPath)
5259
if _, ok := i.cache[normalizedPath]; !ok {
@@ -56,12 +63,16 @@ func (i *incidentsCache) Add(path string, value CacheValue) {
5663
}
5764

5865
func (i *incidentsCache) Delete(path string) {
66+
i.mutex.Lock()
67+
defer i.mutex.Unlock()
5968
normalizedPath := normalizePath(path)
6069
i.logger.V(8).Info("deleting cache entry for path", "path", path, "normalizedPath", normalizedPath)
6170
delete(i.cache, normalizedPath)
6271
}
6372

6473
func (i *incidentsCache) Keys() []string {
74+
i.mutex.RLock()
75+
defer i.mutex.RUnlock()
6576
keys := make([]string, 0, len(i.cache))
6677
for k := range i.cache {
6778
keys = append(keys, k)
@@ -70,13 +81,22 @@ func (i *incidentsCache) Keys() []string {
7081
}
7182

7283
func (i *incidentsCache) Entries() map[string][]CacheValue {
73-
return i.cache
84+
i.mutex.RLock()
85+
defer i.mutex.RUnlock()
86+
// make sure we never return a reference to original map or any of its slices
87+
clone := make(map[string][]CacheValue, len(i.cache))
88+
for k, v := range i.cache {
89+
clonedV := make([]CacheValue, len(v))
90+
copy(clonedV, v)
91+
clone[k] = clonedV
92+
}
93+
return clone
7494
}
7595

7696
func normalizePath(path string) string {
7797
cleanedPath := filepath.Clean(path)
7898
volumeName := filepath.VolumeName(cleanedPath)
79-
// make sure all volume names are lowercase
99+
// make sure all volume names are uppercase
80100
if volumeName != "" {
81101
cleanedPath = strings.ToUpper(volumeName) + cleanedPath[len(volumeName):]
82102
}

kai_analyzer_rpc/pkg/service/pipe_analyzer.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func NewPipeAnalyzer(ctx context.Context, limitIncidents, limitCodeSnips, contex
8080
discoveryCache: []konveyor.RuleSet{},
8181
discoveryCacheMutex: sync.Mutex{},
8282
cache: NewIncidentsCache(l),
83-
cacheMutex: sync.RWMutex{},
8483
location: location,
8584
contextLines: contextLines,
8685
rules: rules,

0 commit comments

Comments
 (0)