diff --git a/src/html/template/exec_test.go b/src/html/template/exec_test.go index cd6b78a1a9..7d1bef1782 100644 --- a/src/html/template/exec_test.go +++ b/src/html/template/exec_test.go @@ -1720,6 +1720,8 @@ 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 { @@ -1777,6 +1779,39 @@ func TestRecursiveExecute(t *testing.T) { } } +// recursiveInvoker is for TestRecursiveExecuteViaMethod. +type recursiveInvoker struct { + t *testing.T + tmpl *Template +} + +func (r *recursiveInvoker) Recur() (string, error) { + var sb strings.Builder + if err := r.tmpl.ExecuteTemplate(&sb, "subroutine", nil); err != nil { + r.t.Fatal(err) + } + return sb.String(), nil +} + +func TestRecursiveExecuteViaMethod(t *testing.T) { + tmpl := New("") + top, err := tmpl.New("x.html").Parse(`{{.Recur}}`) + if err != nil { + t.Fatal(err) + } + _, err = tmpl.New("subroutine").Parse(``) + if err != nil { + t.Fatal(err) + } + r := &recursiveInvoker{ + t: t, + tmpl: tmpl, + } + if err := top.Execute(io.Discard, r); err != nil { + t.Fatal(err) + } +} + // Issue 43295. func TestTemplateFuncsAfterClone(t *testing.T) { s := `{{ f . }}` diff --git a/src/html/template/template.go b/src/html/template/template.go index 1ff7e1f7a0..69312d36fd 100644 --- a/src/html/template/template.go +++ b/src/html/template/template.go @@ -11,7 +11,6 @@ import ( "os" "path" "path/filepath" - "reflect" "sync" "text/template" "text/template/parse" @@ -36,20 +35,18 @@ var escapeOK = fmt.Errorf("template escaped correctly") // nameSpace is the data structure shared by all templates in an association. type nameSpace struct { - mu sync.RWMutex + mu sync.Mutex set map[string]*Template escaped bool esc escaper - // The original functions, before wrapping. - funcMap FuncMap } // Templates returns a slice of the templates associated with t, including t // itself. func (t *Template) Templates() []*Template { ns := t.nameSpace - ns.mu.RLock() - defer ns.mu.RUnlock() + ns.mu.Lock() + defer ns.mu.Unlock() // Return a slice so we don't expose the map. m := make([]*Template, 0, len(ns.set)) for _, v := range ns.set { @@ -87,8 +84,8 @@ func (t *Template) checkCanParse() error { if t == nil { return nil } - t.nameSpace.mu.RLock() - defer t.nameSpace.mu.RUnlock() + t.nameSpace.mu.Lock() + defer t.nameSpace.mu.Unlock() if t.nameSpace.escaped { return fmt.Errorf("html/template: cannot Parse after Execute") } @@ -97,16 +94,6 @@ func (t *Template) checkCanParse() error { // escape escapes all associated templates. func (t *Template) escape() error { - t.nameSpace.mu.RLock() - escapeErr := t.escapeErr - t.nameSpace.mu.RUnlock() - if escapeErr != nil { - if escapeErr == escapeOK { - return nil - } - return escapeErr - } - t.nameSpace.mu.Lock() defer t.nameSpace.mu.Unlock() t.nameSpace.escaped = true @@ -134,8 +121,6 @@ func (t *Template) Execute(wr io.Writer, data interface{}) error { if err := t.escape(); err != nil { return err } - t.nameSpace.mu.RLock() - defer t.nameSpace.mu.RUnlock() return t.text.Execute(wr, data) } @@ -151,8 +136,6 @@ func (t *Template) ExecuteTemplate(wr io.Writer, name string, data interface{}) if err != nil { return err } - t.nameSpace.mu.RLock() - defer t.nameSpace.mu.RUnlock() return tmpl.text.Execute(wr, data) } @@ -160,27 +143,13 @@ func (t *Template) ExecuteTemplate(wr io.Writer, name string, data interface{}) // is escaped, or returns an error if it cannot be. It returns the named // template. func (t *Template) lookupAndEscapeTemplate(name string) (tmpl *Template, err error) { - t.nameSpace.mu.RLock() - tmpl = t.set[name] - var escapeErr error - if tmpl != nil { - escapeErr = tmpl.escapeErr - } - t.nameSpace.mu.RUnlock() - - if tmpl == nil { - return nil, fmt.Errorf("html/template: %q is undefined", name) - } - if escapeErr != nil { - if escapeErr != escapeOK { - return nil, escapeErr - } - return tmpl, nil - } - t.nameSpace.mu.Lock() defer t.nameSpace.mu.Unlock() t.nameSpace.escaped = true + tmpl = t.set[name] + if tmpl == nil { + return nil, fmt.Errorf("html/template: %q is undefined", name) + } if tmpl.escapeErr != nil && tmpl.escapeErr != escapeOK { return nil, tmpl.escapeErr } @@ -286,13 +255,6 @@ func (t *Template) Clone() (*Template, error) { } ns := &nameSpace{set: make(map[string]*Template)} ns.esc = makeEscaper(ns) - if t.nameSpace.funcMap != nil { - ns.funcMap = make(FuncMap, len(t.nameSpace.funcMap)) - for name, fn := range t.nameSpace.funcMap { - ns.funcMap[name] = fn - } - } - wrapFuncs(ns, textClone, ns.funcMap) ret := &Template{ nil, textClone, @@ -307,13 +269,12 @@ func (t *Template) Clone() (*Template, error) { return nil, fmt.Errorf("html/template: cannot Clone %q after it has executed", t.Name()) } x.Tree = x.Tree.Copy() - tc := &Template{ + ret.set[name] = &Template{ nil, x, x.Tree, ret.nameSpace, } - ret.set[name] = tc } // Return the template associated with the name of this template. return ret.set[ret.Name()], nil @@ -382,43 +343,10 @@ type FuncMap map[string]interface{} // type. However, it is legal to overwrite elements of the map. The return // value is the template, so calls can be chained. func (t *Template) Funcs(funcMap FuncMap) *Template { - t.nameSpace.mu.Lock() - if t.nameSpace.funcMap == nil { - t.nameSpace.funcMap = make(FuncMap, len(funcMap)) - } - for name, fn := range funcMap { - t.nameSpace.funcMap[name] = fn - } - t.nameSpace.mu.Unlock() - - wrapFuncs(t.nameSpace, t.text, funcMap) + t.text.Funcs(template.FuncMap(funcMap)) return t } -// wrapFuncs records the functions with text/template. We wrap them to -// unlock the nameSpace. See TestRecursiveExecute for a test case. -func wrapFuncs(ns *nameSpace, textTemplate *template.Template, funcMap FuncMap) { - if len(funcMap) == 0 { - return - } - tfuncs := make(template.FuncMap, len(funcMap)) - for name, fn := range funcMap { - fnv := reflect.ValueOf(fn) - wrapper := func(args []reflect.Value) []reflect.Value { - ns.mu.RUnlock() - defer ns.mu.RLock() - if fnv.Type().IsVariadic() { - return fnv.CallSlice(args) - } else { - return fnv.Call(args) - } - } - wrapped := reflect.MakeFunc(fnv.Type(), wrapper) - tfuncs[name] = wrapped.Interface() - } - textTemplate.Funcs(tfuncs) -} - // Delims sets the action delimiters to the specified strings, to be used in // subsequent calls to Parse, ParseFiles, or ParseGlob. Nested template // definitions will inherit the settings. An empty delimiter stands for the @@ -432,8 +360,8 @@ func (t *Template) Delims(left, right string) *Template { // Lookup returns the template with the given name that is associated with t, // or nil if there is no such template. func (t *Template) Lookup(name string) *Template { - t.nameSpace.mu.RLock() - defer t.nameSpace.mu.RUnlock() + t.nameSpace.mu.Lock() + defer t.nameSpace.mu.Unlock() return t.set[name] }