From 0739cf234853e8c353d5ce98b7135f5b5a3eeb03 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Fri, 15 Jan 2016 16:28:47 -0500 Subject: [PATCH] provider/template: fix race causing panic in template_file The render code path in `template_file` was doing unsynchronized access to a shared mapping of functions in `config.Func`. This caused a race condition that was most often triggered when a `template_file` had a `count` of more than one, and expressed itself as a panic in the plugin followed by a cascade of "unexpected EOF" errors through the plugin system. Here, we simply turn the FuncMap from shared state into a generated value, which avoids the race. We do more re-initialization of the data structure, but the performance implications are minimal, and we can always revisit with a perf pass later now that the race is fixed. --- .../template/resource_template_file.go | 2 +- .../template/resource_template_file_test.go | 24 +++++++++++++++++++ config/interpolate_funcs.go | 6 ++--- config/raw_config.go | 2 +- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/builtin/providers/template/resource_template_file.go b/builtin/providers/template/resource_template_file.go index fd0808828cc2..554fa18bafef 100644 --- a/builtin/providers/template/resource_template_file.go +++ b/builtin/providers/template/resource_template_file.go @@ -155,7 +155,7 @@ func execute(s string, vars map[string]interface{}) (string, error) { cfg := lang.EvalConfig{ GlobalScope: &ast.BasicScope{ VarMap: varmap, - FuncMap: config.Funcs, + FuncMap: config.Funcs(), }, } diff --git a/builtin/providers/template/resource_template_file_test.go b/builtin/providers/template/resource_template_file_test.go index 91882d9d37d1..9f54858dde55 100644 --- a/builtin/providers/template/resource_template_file_test.go +++ b/builtin/providers/template/resource_template_file_test.go @@ -2,6 +2,7 @@ package template import ( "fmt" + "sync" "testing" r "github.com/hashicorp/terraform/helper/resource" @@ -76,6 +77,29 @@ func TestTemplateVariableChange(t *testing.T) { }) } +// This test covers a panic due to config.Func formerly being a +// shared map, causing multiple template_file resources to try and +// accessing it parallel during their lang.Eval() runs. +// +// Before fix, test fails under `go test -race` +func TestTemplateSharedMemoryRace(t *testing.T) { + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + go func(wg sync.WaitGroup, t *testing.T, i int) { + wg.Add(1) + out, err := execute("don't panic!", map[string]interface{}{}) + if err != nil { + t.Fatalf("err: %s", err) + } + if out != "don't panic!" { + t.Fatalf("bad output: %s", out) + } + wg.Done() + }(wg, t, i) + } + wg.Wait() +} + func testTemplateConfig(template, vars string) string { return fmt.Sprintf(` resource "template_file" "t0" { diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 0ca16b56cd6a..487efdd54b17 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -20,10 +20,8 @@ import ( ) // Funcs is the mapping of built-in functions for configuration. -var Funcs map[string]ast.Function - -func init() { - Funcs = map[string]ast.Function{ +func Funcs() map[string]ast.Function { + return map[string]ast.Function{ "cidrhost": interpolationFuncCidrHost(), "cidrnetmask": interpolationFuncCidrNetmask(), "cidrsubnet": interpolationFuncCidrSubnet(), diff --git a/config/raw_config.go b/config/raw_config.go index ebb9f18dc67f..2a66288d1d5e 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -300,7 +300,7 @@ type gobRawConfig struct { // langEvalConfig returns the evaluation configuration we use to execute. func langEvalConfig(vs map[string]ast.Variable) *lang.EvalConfig { funcMap := make(map[string]ast.Function) - for k, v := range Funcs { + for k, v := range Funcs() { funcMap[k] = v } funcMap["lookup"] = interpolationFuncLookup(vs)