1
0
mirror of https://github.com/golang/go synced 2024-11-26 14:36:52 -07:00

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>
This commit is contained in:
Ian Lance Taylor 2021-05-03 16:32:52 -07:00
parent 731a015ab8
commit 496d7c6914
4 changed files with 51 additions and 7 deletions

View File

@ -1720,8 +1720,6 @@ var v = "v";
` `
func TestEscapeRace(t *testing.T) { func TestEscapeRace(t *testing.T) {
t.Skip("this test currently fails with -race; see issue #39807")
tmpl := New("") tmpl := New("")
_, err := tmpl.New("templ.html").Parse(raceText) _, err := tmpl.New("templ.html").Parse(raceText)
if err != nil { if err != nil {

View File

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

View File

@ -12,6 +12,7 @@ import (
"io" "io"
"reflect" "reflect"
"strings" "strings"
"sync"
"testing" "testing"
) )
@ -1732,3 +1733,40 @@ func TestIssue43065(t *testing.T) {
t.Errorf("%s", err) 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()
}

View File

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