Skip to content

Commit

Permalink
provider/template: fix race causing panic in template_file
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
phinze committed Jan 15, 2016
1 parent ead4865 commit 0739cf2
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
2 changes: 1 addition & 1 deletion builtin/providers/template/resource_template_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
}

Expand Down
24 changes: 24 additions & 0 deletions builtin/providers/template/resource_template_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package template

import (
"fmt"
"sync"
"testing"

r "github.com/hashicorp/terraform/helper/resource"
Expand Down Expand Up @@ -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" {
Expand Down
6 changes: 2 additions & 4 deletions config/interpolate_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion config/raw_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 0739cf2

Please sign in to comment.