{{block "a" .}}a{{end}}

From cd2c9df7612795cad5b56cabe5ec29c7771db5fe Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Fri, 14 Oct 2016 00:59:19 -0700 Subject: [PATCH] html/template: fix Clone so that t.Lookup(t.Name()) yields t Template.escape makes the assumption that t.Lookup(t.Name()) is t (escapeTemplate looks up the associated template by name and sets escapeErr appropriately). This assumption did not hold for a Cloned template, because the template associated with t.Name() was a second copy of the original. Add a test for the assumption that t.Lookup(t.Name()) == t. One effect of this broken assumption was #16101: parallel Executes racily accessed the template namespace because each Execute call saw t.escapeErr == nil and re-escaped the template concurrently with read accesses occurring outside the namespace mutex. Add a test for this race. Related to #12996 and CL 16104. Fixes #16101 Change-Id: I59831d0847abbabb4ef9135f2912c6ce982f9837 Reviewed-on: https://go-review.googlesource.com/31092 Run-TryBot: Rob Pike TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/html/template/clone_test.go | 35 +++++++++++++++++++++++++++++++++ src/html/template/template.go | 3 +++ 2 files changed, 38 insertions(+) diff --git a/src/html/template/clone_test.go b/src/html/template/clone_test.go index d7c62fa399..069064c98b 100644 --- a/src/html/template/clone_test.go +++ b/src/html/template/clone_test.go @@ -8,6 +8,7 @@ import ( "bytes" "errors" "io/ioutil" + "sync" "testing" "text/template/parse" ) @@ -194,3 +195,37 @@ func TestFuncMapWorksAfterClone(t *testing.T) { t.Errorf("clone error message mismatch want %q got %q", wantErr, gotErr) } } + +// https://golang.org/issue/16101 +func TestTemplateCloneExecuteRace(t *testing.T) { + const ( + input = `{{block "a" .}}a{{end}}{{block "b" .}}b{{end}}` + overlay = `{{define "b"}}A{{end}}` + ) + outer := Must(New("outer").Parse(input)) + tmpl := Must(Must(outer.Clone()).Parse(overlay)) + + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + if err := tmpl.Execute(ioutil.Discard, "data"); err != nil { + panic(err) + } + } + }() + } + wg.Wait() +} + +func TestTemplateCloneLookup(t *testing.T) { + // Template.escape makes an assumption that the template associated + // with t.Name() is t. Check that this holds. + tmpl := Must(New("x").Parse("a")) + tmpl = Must(tmpl.Clone()) + if tmpl.Lookup(tmpl.Name()) != tmpl { + t.Error("after Clone, tmpl.Lookup(tmpl.Name()) != tmpl") + } +} diff --git a/src/html/template/template.go b/src/html/template/template.go index 063e46d6bf..d5e195ff69 100644 --- a/src/html/template/template.go +++ b/src/html/template/template.go @@ -240,6 +240,9 @@ func (t *Template) Clone() (*Template, error) { ret.set[ret.Name()] = ret for _, x := range textClone.Templates() { name := x.Name() + if name == ret.Name() { + continue + } src := t.set[name] if src == nil || src.escapeErr != nil { return nil, fmt.Errorf("html/template: cannot Clone %q after it has executed", t.Name()) -- 2.48.1