From: Rob Pike <r@golang.org>
Date: Mon, 4 May 2015 20:09:31 +0000 (-0700)
Subject: text/template: shut down lexing goroutine on error
X-Git-Tag: go1.5beta1~749
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=64c39a3093db3976201697bb817d5705afc66bed;p=gostls13.git

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 fe77b3afbe..762d085e68 100644
--- 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 a1cda19e67..be551d8780 100644
--- 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 f4daa37954..6eb303801b 100644
--- 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)