text/template: shut down lexing goroutine on error

When a parse error occurred, the lexing goroutine would lay idle.
It's not likely a problem but if the program is for some reason
accepting badly formed data repeatedly, it's wasteful.

The solution is easy: Just drain the input on error. We know this
will succeed because the input is always a string and is therefore
guaranteed finite.

With debugging prints in the package tests I've shown this is effective,
shutting down 79 goroutines that would otherwise linger, out of 123 total.

Fixes #10574.

Change-Id: I8aa536e327b219189a7e7f604a116fa562ae1c39
Reviewed-on: https://go-review.googlesource.com/9658
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/text/template/parse/lex.go b/src/text/template/parse/lex.go
index fe77b3afbebde97d555e16cfb7c6b6338470e3e2..762d085e685310e2ddc903c7d4a22da1a7be730b 100644 (file)
--- a/src/text/template/parse/lex.go
+++ b/src/text/template/parse/lex.go
@@ -167,12 +167,23 @@ func (l *lexer) errorf(format string, args ...interface{}) stateFn {
 }
 
 // nextItem returns the next item from the input.
+// Called by the parser, not in the lexing goroutine.
 func (l *lexer) nextItem() item {
        item := <-l.items
        l.lastPos = item.pos
        return item
 }
 
+// drain drains the output so the lexing goroutine will exit.
+// Called by the parser, not in the lexing goroutine.
+func (l *lexer) drain() {
+       if l == nil {
+               return
+       }
+       for range l.items {
+       }
+}
+
 // lex creates a new scanner for the input string.
 func lex(name, input, left, right string) *lexer {
        if left == "" {
@@ -197,6 +208,7 @@ func (l *lexer) run() {
        for l.state = lexText; l.state != nil; {
                l.state = l.state(l)
        }
+       close(l.items)
 }
 
 // state functions

diff --git a/src/text/template/parse/lex_test.go b/src/text/template/parse/lex_test.go
index a1cda19e6757e231956d2611e18730d273693a07..be551d87803228916a661c6333b07433e517f95a 100644 (file)
--- a/src/text/template/parse/lex_test.go
+++ b/src/text/template/parse/lex_test.go
@@ -466,3 +466,31 @@ func TestPos(t *testing.T) {
                }
        }
 }
+
+// Test that an error shuts down the lexing goroutine.
+func TestShutdown(t *testing.T) {
+       // We need to duplicate template.Parse here to hold on to the lexer.
+       const text = "erroneous{{define}}{{else}}1234"
+       lexer := lex("foo", text, "{{", "}}")
+       _, err := New("root").parseLexer(lexer, text)
+       if err == nil {
+               t.Fatalf("expected error")
+       }
+       // The error should have drained the input. Therefore, the lexer should be shut down.
+       token, ok := <-lexer.items
+       if ok {
+               t.Fatalf("input was not drained; got %v", token)
+       }
+}
+
+// parseLexer is a local version of parse that lets us pass in the lexer instead of building it.
+// We expect an error, so the tree set and funcs list are explicitly nil.
+func (t *Tree) parseLexer(lex *lexer, text string) (tree *Tree, err error) {
+       defer t.recover(&err)
+       t.ParseName = t.Name
+       t.startParse(nil, lex)
+       t.parse(nil)
+       t.add(nil)
+       t.stopParse()
+       return t, nil
+}

diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go
index f4daa379547ef2f0e31e71ff73ea3999a315e61b..6eb303801b7becd5f116c8047a250f0ab71a67fc 100644 (file)
--- a/src/text/template/parse/parse.go
+++ b/src/text/template/parse/parse.go
@@ -196,6 +196,7 @@ func (t *Tree) recover(errp *error) {
                        panic(e)
                }
                if t != nil {
+                       t.lex.drain()
                        t.stopParse()
                }
                *errp = e.(error)