Backport js vm fixes from 6.x to 5.21 (#4053)

Signed-off-by: Eric Chlebek <eric@sensu.io>

CHANGELOG.md

@@ -9,6 +9,7 @@ Versioning](http://semver.org/spec/v2.0.0.html).
 
 ### Fixed
 - Close the response body when done reading from it while downloading assets.
+- Fixed a bug where event filter or asset filter execution could cause a crash.
 
 ## [5.21.2] - 2020-08-31
 

js/js.go

@@ -47,14 +47,10 @@ func ParseExpressions(expressions []string) error {
 	return nil
 }
 
-func newOttoVM(assets JavascriptAssets) (*otto.Otto, error) {
+func newOttoVM(assets JavascriptAssets, key string) (*otto.Otto, error) {
 	ottoOnce.Do(func() {
 		ottoCache = newVMCache()
 	})
-	key := ""
-	if assets != nil {
-		key = assets.Key()
-	}
 	vm := ottoCache.Acquire(key)
 	if vm != nil {
 		return vm, nil
@@ -72,6 +68,13 @@ func newOttoVM(assets JavascriptAssets) (*otto.Otto, error) {
 	return ottoCache.Acquire(key), nil
 }
 
+func releaseOttoVM(key string) {
+	ottoOnce.Do(func() {
+		panic("releaseOttoVM called before newOttoVM")
+	})
+	ottoCache.Dispose(key)
+}
+
 func addAssets(vm *otto.Otto, assets JavascriptAssets) error {
 	scripts, err := assets.Scripts()
 	if err != nil {
@@ -122,10 +125,15 @@ func addTimeFuncs(vm *otto.Otto) error {
 // If scripts is non-nil, then the scripts will be evaluated in the
 // expression's runtime context before the expression is evaluated.
 func Evaluate(expr string, parameters interface{}, assets JavascriptAssets) (bool, error) {
-	jsvm, err := newOttoVM(assets)
+	key := ""
+	if assets != nil {
+		key = assets.Key()
+	}
+	jsvm, err := newOttoVM(assets, key)
 	if err != nil {
 		return false, err
 	}
+	defer releaseOttoVM(key)
 	if params, ok := parameters.(map[string]interface{}); ok {
 		for name, value := range params {
 			if err := jsvm.Set(name, value); err != nil {
@@ -140,6 +148,34 @@ func Evaluate(expr string, parameters interface{}, assets JavascriptAssets) (boo
 	return value.ToBoolean()
 }
 
+// EvalPredicateWithVM is like Evaluate, but allows passing a vm explicitly.
+func EvalPredicateWithVM(vm *otto.Otto, parameters map[string]interface{}, expr string) (bool, error) {
+	for name, value := range parameters {
+		if err := vm.Set(name, value); err != nil {
+			return false, err
+		}
+	}
+	value, err := vm.Run(expr)
+	if err != nil {
+		return false, err
+	}
+	return value.ToBoolean()
+}
+
+// WithOttoVM provides a context manager for working with cached VMs.
+func WithOttoVM(assets JavascriptAssets, fn func(vm *otto.Otto) error) error {
+	key := ""
+	if assets != nil {
+		key = assets.Key()
+	}
+	jsvm, err := newOttoVM(assets, key)
+	if err != nil {
+		return err
+	}
+	defer releaseOttoVM(key)
+	return fn(jsvm)
+}
+
 // EntityFilterResult is returned by EvaluateEntityFilters
 type EntityFilterResult struct {
 	Value bool
@@ -159,10 +195,11 @@ type EntityFilterResult struct {
 // If the function cannot set up a javascript VM, or has issues setting vars,
 // then the function returns a nil slice and a non-nil error.
 func MatchEntities(expressions []string, entities []interface{}) ([]bool, error) {
-	jsvm, err := newOttoVM(nil)
+	jsvm, err := newOttoVM(nil, "")
 	if err != nil {
 		return nil, fmt.Errorf("error evaluating entity filters: %s", err)
 	}
+	defer releaseOttoVM("")
 	scripts := make([]*otto.Script, 0, len(expressions))
 	for _, expr := range expressions {
 		script, err := jsvm.Compile("", expr)

js/vm_cache.go

@@ -1,6 +1,7 @@
 package js
 
 import (
+	"fmt"
 	"sync"
 
 	time "github.com/echlebek/timeproxy"
@@ -17,22 +18,21 @@ const (
 )
 
 // vmCache provides an internal mechanism for caching javascript contexts
-// according to which assets are loaded into them. Javascrip contexts which
+// according to which assets are loaded into them. Javascript contexts which
 // are not used for cacheMaxAge are disposed of.
 type vmCache struct {
-	vms  map[string]*cacheValue
+	vms  sync.Map
 	done chan struct{}
-	sync.Mutex
 }
 
 type cacheValue struct {
 	lastRead int64
+	mu       sync.Mutex
 	vm       *otto.Otto
 }
 
 func newVMCache() *vmCache {
 	cache := &vmCache{
-		vms:  make(map[string]*cacheValue),
 		done: make(chan struct{}),
 	}
 	go cache.reapLoop()
@@ -58,34 +58,44 @@ func (c *vmCache) reapLoop() {
 }
 
 func (c *vmCache) reap() {
-	c.Lock()
-	for k, v := range c.vms {
-		valueTime := time.Unix(v.lastRead, 0)
-		if valueTime.Before(time.Now().Add(-cacheMaxAge)) {
-			delete(c.vms, k)
+	c.vms.Range(func(key, value interface{}) bool {
+		obj := value.(*cacheValue)
+		obj.mu.Lock()
+		defer obj.mu.Unlock()
+		valueTime := time.Unix(obj.lastRead, 0)
+		if time.Since(valueTime) > cacheMaxAge {
+			c.vms.Delete(key)
 		}
-	}
-	defer c.Unlock()
+		return true
+	})
 }
 
 // Acquire gets a VM from the cache. It is a copy of the cached value.
+// The cache item is locked while in use.
+// Users must call Dispose with the key after Acquire.
 func (c *vmCache) Acquire(key string) *otto.Otto {
-	c.Lock()
-	defer c.Unlock()
-	val, ok := c.vms[key]
+	val, ok := c.vms.Load(key)
 	if !ok {
 		return nil
 	}
-	if val.vm == nil {
-		return nil
+	obj := val.(*cacheValue)
+	obj.mu.Lock()
+	obj.lastRead = time.Now().Unix()
+	return obj.vm.Copy()
+}
+
+// Dispose releases the lock on the cache item.
+func (c *vmCache) Dispose(key string) {
+	val, ok := c.vms.Load(key)
+	if !ok {
+		panic(fmt.Sprintf("dispose called on %q, but not found", key))
 	}
-	return val.vm.Copy()
+	obj := val.(*cacheValue)
+	obj.mu.Unlock()
 }
 
 // Init initializes the value in the cache.
 func (c *vmCache) Init(key string, vm *otto.Otto) {
-	c.Lock()
-	defer c.Unlock()
 	val := &cacheValue{lastRead: time.Now().Unix(), vm: vm}
-	c.vms[key] = val
+	c.vms.Store(key, val)
 }