diff options
| author | Caleb Spare <cespare@gmail.com> | 2016-10-14 00:59:19 -0700 |
|---|---|---|
| committer | Rob Pike <r@golang.org> | 2016-10-17 00:35:20 +0000 |
| commit | cd2c9df7612795cad5b56cabe5ec29c7771db5fe (patch) | |
| tree | c7246b27dc2b0248b4c58d28f157f7e73db9b703 /src/html | |
| parent | d8cbc2c918f68e8ca5992e68fed052a0e52a8e67 (diff) | |
| download | go-cd2c9df7612795cad5b56cabe5ec29c7771db5fe.tar.xz | |
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 <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Diffstat (limited to 'src/html')
| -rw-r--r-- | src/html/template/clone_test.go | 35 | ||||
| -rw-r--r-- | src/html/template/template.go | 3 |
2 files changed, 38 insertions, 0 deletions
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 = `<title>{{block "a" .}}a{{end}}</title><body>{{block "b" .}}b{{end}}<body>` + 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()) |