Browse files

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>
  • Loading branch information...
1 parent 5a828cf commit 64c39a3093db3976201697bb817d5705afc66bed @robpike robpike committed May 4, 2015
Showing with 41 additions and 0 deletions.
  1. +12 −0 src/text/template/parse/lex.go
  2. +28 −0 src/text/template/parse/lex_test.go
  3. +1 −0 src/text/template/parse/parse.go
View
12 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 {
@gravis
gravis added a note May 6, 2015

how l could be nil here?

@robpike
robpike added a note May 7, 2015

It can't. Well spotted.

This was added during testing but is no longer necessary. See CL 9841.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ 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
View
28 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
+}
View
1 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)

0 comments on commit 64c39a3

Please sign in to comment.