text/template: add lock for Template.tmpl to fix data race

This adds a new lock protecting "tmpl".

This is a copy of https://golang.org/cl/257817 by Andreas Fleig,
updated for current tip, and updated to start running the
html/template TestEscapeRace test.

Thanks to @bep for providing the test case.

Fixes #39807

Change-Id: Ic8874484290283a49116812eeaffb8608346dc70
Reviewed-on: https://go-review.googlesource.com/c/go/+/316669
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/html/template/exec_test.go b/src/html/template/exec_test.go
index 7d1bef1782a9cc0be2a97179acb080a5ab7afad3..888587335de112cd5637f0bfe4e94d3213126c43 100644 (file)
--- a/src/html/template/exec_test.go
+++ b/src/html/template/exec_test.go
@@ -1720,8 +1720,6 @@ var v = "v";
 `
 
 func TestEscapeRace(t *testing.T) {
-       t.Skip("this test currently fails with -race; see issue #39807")
-
        tmpl := New("")
        _, err := tmpl.New("templ.html").Parse(raceText)
        if err != nil {

diff --git a/src/text/template/exec.go b/src/text/template/exec.go
index f1305a29a0de31d739642e5a7cbb4205b58df2d0..5ad3b4ec582c411fe7d4d3819c03a82f5c0041f9 100644 (file)
--- a/src/text/template/exec.go
+++ b/src/text/template/exec.go
@@ -179,10 +179,7 @@ func errRecover(errp *error) {
 // A template may be executed safely in parallel, although if parallel
 // executions share a Writer the output may be interleaved.
 func (t *Template) ExecuteTemplate(wr io.Writer, name string, data interface{}) error {
-       var tmpl *Template
-       if t.common != nil {
-               tmpl = t.tmpl[name]
-       }
+       tmpl := t.Lookup(name)
        if tmpl == nil {
                return fmt.Errorf("template: no template %q associated with template %q", name, t.name)
        }
@@ -230,6 +227,8 @@ func (t *Template) DefinedTemplates() string {
                return ""
        }
        var b strings.Builder
+       t.muTmpl.RLock()
+       defer t.muTmpl.RUnlock()
        for name, tmpl := range t.tmpl {
                if tmpl.Tree == nil || tmpl.Root == nil {
                        continue
@@ -401,7 +400,7 @@ func (s *state) walkRange(dot reflect.Value, r *parse.RangeNode) {
 
 func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) {
        s.at(t)
-       tmpl := s.tmpl.tmpl[t.Name]
+       tmpl := s.tmpl.Lookup(t.Name)
        if tmpl == nil {
                s.errorf("template %q not defined", t.Name)
        }

diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index 255b111b34bf3c9676b7f8fe8df7c1e8f7160907..e73fce4fa80edfb46e405cc1843297a3d9ed51f4 100644 (file)
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -12,6 +12,7 @@ import (
        "io"
        "reflect"
        "strings"
+       "sync"
        "testing"
 )
 
@@ -1732,3 +1733,40 @@ func TestIssue43065(t *testing.T) {
                t.Errorf("%s", err)
        }
 }
+
+// Issue 39807: data race in html/template & text/template
+func TestIssue39807(t *testing.T) {
+       var wg sync.WaitGroup
+
+       tplFoo, err := New("foo").Parse(`{{ template "bar" . }}`)
+       if err != nil {
+               t.Error(err)
+       }
+
+       tplBar, err := New("bar").Parse("bar")
+       if err != nil {
+               t.Error(err)
+       }
+
+       gofuncs := 10
+       numTemplates := 10
+
+       for i := 1; i <= gofuncs; i++ {
+               wg.Add(1)
+               go func() {
+                       defer wg.Done()
+                       for j := 0; j < numTemplates; j++ {
+                               _, err := tplFoo.AddParseTree(tplBar.Name(), tplBar.Tree)
+                               if err != nil {
+                                       t.Error(err)
+                               }
+                               err = tplFoo.Execute(io.Discard, nil)
+                               if err != nil {
+                                       t.Error(err)
+                               }
+                       }
+               }()
+       }
+
+       wg.Wait()
+}

diff --git a/src/text/template/template.go b/src/text/template/template.go
index ec26eb4c50a7a4a17d2450b18cbdfebd31af01f1..fd74d45e9b1c6df9a0bf45206eba1d040dd0d3b9 100644 (file)
--- a/src/text/template/template.go
+++ b/src/text/template/template.go
@@ -13,6 +13,7 @@ import (
 // common holds the information shared by related templates.
 type common struct {
        tmpl   map[string]*Template // Map from name to defined templates.
+       muTmpl sync.RWMutex         // protects tmpl
        option option
        // We use two maps, one for parsing and one for execution.
        // This separation makes the API cleaner since it doesn't
@@ -88,6 +89,8 @@ func (t *Template) Clone() (*Template, error) {
        if t.common == nil {
                return nt, nil
        }
+       t.muTmpl.RLock()
+       defer t.muTmpl.RUnlock()
        for k, v := range t.tmpl {
                if k == t.name {
                        nt.tmpl[t.name] = nt
@@ -124,6 +127,8 @@ func (t *Template) copy(c *common) *Template {
 // its definition. If it has been defined and already has that name, the existing
 // definition is replaced; otherwise a new template is created, defined, and returned.
 func (t *Template) AddParseTree(name string, tree *parse.Tree) (*Template, error) {
+       t.muTmpl.Lock()
+       defer t.muTmpl.Unlock()
        t.init()
        nt := t
        if name != t.name {
@@ -142,6 +147,8 @@ func (t *Template) Templates() []*Template {
                return nil
        }
        // Return a slice so we don't expose the map.
+       t.muTmpl.RLock()
+       defer t.muTmpl.RUnlock()
        m := make([]*Template, 0, len(t.tmpl))
        for _, v := range t.tmpl {
                m = append(m, v)
@@ -182,6 +189,8 @@ func (t *Template) Lookup(name string) *Template {
        if t.common == nil {
                return nil
        }
+       t.muTmpl.RLock()
+       defer t.muTmpl.RUnlock()
        return t.tmpl[name]
 }