From 21fcbb39b73310e69d6403a1cfa8b26799cc1355 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 4 Jan 2017 15:29:10 -0500 Subject: [PATCH] Fix race accessing plugin storage map `plugins.GetAll()` was not locking the plugin map when checking if a plugin exists, this can cause a race and potentially a panic if another goroutine is trying to load a plugin into the map at the same time. Also fixes a race during activation where a plugin inserts itself into the plugin map but does not check if something else is already there. This is already checked before trying to activate the plugin, however the map lock is not held for this entire period, so other plugins may be loaded during this time. To fix, before inserting the plugin into the map, check if one with the same name already exists and use that instead. Signed-off-by: Brian Goff --- pkg/plugins/plugins.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 125e6c7d66..861daa3207 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -221,6 +221,10 @@ func loadWithRetry(name string, retry bool) (*Plugin, error) { } storage.Lock() + if pl, exists := storage.plugins[name]; exists { + storage.Unlock() + return pl, pl.activate() + } storage.plugins[name] = pl storage.Unlock() @@ -298,7 +302,10 @@ func GetAll(imp string) ([]*Plugin, error) { chPl := make(chan *plLoad, len(pluginNames)) var wg sync.WaitGroup for _, name := range pluginNames { - if pl, ok := storage.plugins[name]; ok { + storage.Lock() + pl, ok := storage.plugins[name] + storage.Unlock() + if ok { chPl <- &plLoad{pl, nil} continue }