From 8edaa8ad4d97ee3cda2f69b650407c785b0707af Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 12 Nov 2018 18:47:13 +0000 Subject: [PATCH] Fix goroutine leak in lexer/parser. (#4858) When there was an error in the parser, the lexer goroutine was left running. Also make runtime panic test actually test things. Signed-off-by: Brian Brazil --- promql/lex.go | 7 +++++++ promql/parse.go | 1 + promql/parse_test.go | 19 ++++++++++++------- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/promql/lex.go b/promql/lex.go index 01372ae2e..10fb49bc4 100644 --- a/promql/lex.go +++ b/promql/lex.go @@ -434,6 +434,13 @@ func (l *lexer) run() { close(l.items) } +// Release resources used by lexer. +func (l *lexer) close() { + for range l.items { + // Consume. + } +} + // lineComment is the character that starts a line comment. const lineComment = "#" diff --git a/promql/parse.go b/promql/parse.go index 8abf8dbf9..ddf354d2c 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -337,6 +337,7 @@ func (p *parser) recover(errp *error) { *errp = e.(error) } } + p.lex.close() } // expr parses any expression. diff --git a/promql/parse_test.go b/promql/parse_test.go index f27701548..965306af0 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -1581,21 +1581,26 @@ func TestParseSeries(t *testing.T) { } func TestRecoverParserRuntime(t *testing.T) { - var p *parser + p := newParser("foo bar") var err error - defer p.recover(&err) + defer func() { + if err != errUnexpected { + t.Fatalf("wrong error message: %q, expected %q", err, errUnexpected) + } + + if _, ok := <-p.lex.items; ok { + t.Fatalf("lex.items was not closed") + } + }() + defer p.recover(&err) // Cause a runtime panic. var a []int a[123] = 1 - - if err != errUnexpected { - t.Fatalf("wrong error message: %q, expected %q", err, errUnexpected) - } } func TestRecoverParserError(t *testing.T) { - var p *parser + p := newParser("foo bar") var err error e := fmt.Errorf("custom error")