Data race when calling funcs.Create*Funcs functions concurrently

[nandu-vinodan]

I am currently working on using gomplate’s CreateFuncs() to run template execution with text/template.
I am running into DATA_RACE warnings when multiple go routines execute the CreateFuncs function concurrently.

Sample code to reproduce the issue:

type MySetting struct {
	Name string `yaml:"name"`
}
var funcs template.FuncMap
func setFuncMap() {
	funcs = gomplate.CreateFuncs(context.Background(), nil)
}
func runGomplate(templateFile string, settings *MySetting) {
	setFuncMap()
	tmpl := template.Must(template.New(filepath.Base(templateFile)).Funcs(
		funcs).ParseFiles(templateFile))
	var o bytes.Buffer
	err := tmpl.Execute(&o, settings)
	if err != nil {
		fmt.Printf("Error occured %v", err)
		panic(err)
	}
	fmt.Println(o.String())
}
func main() {
	t := "sample.tmpl.yaml"
	s := &MySetting{
		Name: "test",
	}
	for i := 0; i < 5; i++ {
		go runGomplate(t, s)
	}
	time.Sleep(5 * time.Second)
}

Running go run -race main.go gives:

WARNING: DATA RACE
Write at 0x00c000170060 by goroutine 25:
  github.com/hairyhenderson/gomplate/v3/funcs.CreateDataFuncs()
      /Users/nanduvinodan/go/pkg/mod/github.com/hairyhenderson/gomplate/v3@v3.9.0/funcs/data.go:40 +0x5a9
  github.com/hairyhenderson/gomplate/v3.CreateFuncs()
      /Users/nanduvinodan/go/pkg/mod/github.com/hairyhenderson/gomplate/v3@v3.9.0/funcs.go:23 +0x7e
  main.setFuncMap()
      /Users/nanduvinodan/workspace/sample-go/main.go:30 +0x78
  main.runGomplate()
      /Users/nanduvinodan/workspace/sample-go/main.go:34 +0x43
Previous write at 0x00c000170060 by goroutine 24:
  github.com/hairyhenderson/gomplate/v3/funcs.CreateDataFuncs()
      /Users/nanduvinodan/go/pkg/mod/github.com/hairyhenderson/gomplate/v3@v3.9.0/funcs/data.go:40 +0x5a9
  github.com/hairyhenderson/gomplate/v3.CreateFuncs()
      /Users/nanduvinodan/go/pkg/mod/github.com/hairyhenderson/gomplate/v3@v3.9.0/funcs.go:23 +0x7e
  main.setFuncMap()
      /Users/nanduvinodan/workspace/sample-go/main.go:30 +0x78
  main.runGomplate()
      /Users/nanduvinodan/workspace/sample-go/main.go:34 +0x43
Goroutine 25 (running) created at:
  main.main()
      /Users/nanduvinodan/workspace/sample-go/main.go:51 +0xad

I am able to work-around this using a sync.Mutex variable for the setFuncMap() function.
Would like to know if we can also use sync.Once to create once and reuse it everywhere. Wondering if gomplate stores some state information internally which prohibits reusing gomplate.CreateFuncs()'s return value multiple times.

[hairyhenderson]

Hi @nandu-vinodan, thanks for logging this issue!

In theory, gomplate.CreateFuncs should be able to be called concurrently without issue.

Your global funcs variable is the issue. setFuncMap is being called concurrently, and because it modifies funcs, this results in a data race.

It can be reproduced simply with a test like this:

package main

import (
	"testing"
)

var n int

func doSomething() {
	n = 42
}

func TestSomething(t *testing.T) {
	for i := 0; i < 5; i++ {
		go doSomething()
	}
}

$ go test -race race_test.go
==================
WARNING: DATA RACE
Write at 0x000001333890 by goroutine 9:
  command-line-arguments.doSomething()
      /tmp/race_test.go:10 +0x3a

Previous write at 0x000001333890 by goroutine 8:
  command-line-arguments.doSomething()
      /tmp/race_test.go:10 +0x3a

Goroutine 9 (running) created at:
  command-line-arguments.TestSomething()
      /tmp/race_test.go:15 +0x4b
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1193 +0x202

Goroutine 8 (finished) created at:
  command-line-arguments.TestSomething()
      /tmp/race_test.go:15 +0x4b
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1193 +0x202
==================
--- FAIL: TestSomething (0.00s)
    testing.go:1092: race detected during execution of test
FAIL
FAIL    command-line-arguments  0.110s
FAIL

You shouldn't need to call gomplate.CreateFuncs more than once anyway - if you look at the code you'll see it's really just collection of function calls in the funcs package (see the Create*Funcs functions).

The one exception is the CreateDataFuncs function, which takes a *data.Data so that it can wire in the datasource-related functions. It's OK to give it a nil if these aren't useful to you - just know that if you do try to call the datasource function, for example, it'll panic 😉

I hope that helps! I'm going to assume that this solves your issue, and so I'm going to close this issue. If there's something I've missed please feel free to re-open it.

Also if you have any general questions around using gomplate programatically (or otherwise!), please feel free to start a discussion!

[nandu-vinodan]

@hairyhenderson Thank you for responding!

I was able to reproduce the issue even without a global variable:

for i := 0; i < 5; i++ {
    go gomplate.CreateFuncs(context.Background(), nil)
}

You shouldn't need to call gomplate.CreateFuncs more than once anyway - if you look at the code you'll see it's really just collection of function calls in the funcs package (see the Create*Funcs functions).

Thanks for confirming this. I'll invoke once and reuse the funcMap.

[hairyhenderson]

Ahh, my apologies @nandu-vinodan! The issue is indeed in the funcs.Create*Funcs code. For example,

gomplate/funcs/coll.go

Line 34 in 8eeedae

ns.ctx = ctx

Even though ns in that instance is created atomically (with a sync.Once), the ctx is modified. That was a mistake on my part. The irony is that ctx isn't even used in most cases - it was a bit of a failed experiment.

Because there is indeed a race condition, and even though there's no reason for gomplate.CreateFuncs to be called concurrently, I'm going to re-open this issue so I can fix the bug properly.

[hairyhenderson]

One more thing I should mention which I had forgotten - the functions in the funcs package is not intended to be part of the exported API (see doc at top of https://pkg.go.dev/github.com/hairyhenderson/gomplate/v3@v3.9.0/funcs), so it's expected that the API may break.

If I'd known about the internal/ convention when I had started writing gomplate I would've placed it there (and it will probably move to internal/ in gomplate 4.0).

All that to say - I would recommend only calling gomplate.CreateFuncs, and not calling any of the functions in the funcs package. You're already doing this, but I just wanted to reinforce that 🙂

[nandu-vinodan]

Thanks for the quick turnaround @hairyhenderson. #1127 fixes this issue.