]> Cypherpunks repositories - gostls13.git/commitdiff
html/template: fix Clone so that t.Lookup(t.Name()) yields t
authorCaleb Spare <cespare@gmail.com>
Fri, 14 Oct 2016 07:59:19 +0000 (00:59 -0700)
committerRob Pike <r@golang.org>
Mon, 17 Oct 2016 00:35:20 +0000 (00:35 +0000)
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>
src/html/template/clone_test.go
src/html/template/template.go

index d7c62fa3993b042fcc53ed7e03f1f5be5f769695..069064c98b62d9acf7670150d041be809c9bf4d6 100644 (file)
@@ -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")
+       }
+}
index 063e46d6bf3c67f6a2418c383d388ee04accabc7..d5e195ff69d5ef9277f4c955cf96ee2416fe0eff 100644 (file)
@@ -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())