text/template/parse: Parse will leak go routines if parsing fails

[gravis]

(From https://groups.google.com/forum/#!topic/golang-nuts/6nTWBLbAyKo)

When Parse() is called, a new lexer is created, and it will run in the background:

func (t *Tree) Parse(text, leftDelim, rightDelim string, treeSet map[string]*Tree, funcs ...map[string]interface{}) (tree *Tree, err error) {

[...]

t.startParse(funcs, lex(t.Name, text, leftDelim, rightDelim))

[...]

}

(https://github.com/golang/go/blob/master/src/text/template/parse/parse.go#L228)

This will start a go routine :

// lex creates a new scanner for the input string.
func lex(name, input, left, right string) *lexer {
    if left == "" {
        left = leftDelim
    }
    if right == "" {
        right = rightDelim
    }
    l := &lexer{
        name:       name,
        input:      input,
        leftDelim:  left,
        rightDelim: right,
        items:      make(chan item),
    }
    go l.run()
    return l
}

(https://github.com/golang/go/blob/master/src/text/template/parse/lex.go#L191)

Now if the parser fails to parse, it will panic, and the error is recovered :

// recover is the handler that turns panics into returns from the top level of Parse.
func (t *Tree) recover(errp *error) {
    e := recover()
    if e != nil {
        if _, ok := e.(runtime.Error); ok {
            panic(e)
        }
        if t != nil {
            t.stopParse()
        }
        *errp = e.(error)
    }
    return
}

(https://github.com/golang/go/blob/master/src/text/template/parse/parse.go#L191)

which will call t.stopParse():

// stopParse terminates parsing.
func (t *Tree) stopParse() {
    t.lex = nil
    t.vars = nil
    t.funcs = nil
}

The lexer should be GC, except it's still running in the background, because no parser is there to pull on the channel.
I think the lex should have a abort() method to end the routine cleanly, otherwise each time the parser is called and failing, we leave a running lexer behind.

I will create a pull request to fix this, as I have already some code for that.
ps: Minux has confirmed the bug http://play.golang.org/p/qTQXuT3ktV

[ianlancetaylor]

@griesemer

[gravis]

Patch is ready, I'm stuck here: "Thanks, your CLA has been submitted."
I hope it won't be too long.

[bradfitz]

Did you do a corporate CLA then I assume? Individual CLAs are instantly recognized by Gerrit.

If you did corporate, email me (@golang.org) the company name and I'll look into it.

[gravis]

I did a corporate first, saw the big form, and decided to go with the individual one.
Sounds like something went wrong then :(
I just had the message "Thanks, your CLA has been submitted." and nothing more.
It would be nice to unblock my account if needed.
Thanks
​

[bradfitz]

What makes you think your account is blocked? Email me the answer. Let's not discuss in the bug tracker. Try just mailing the CL with codereview again.

[gravis]

Ok, nailed it.
I mixed up with my corporate account.
It's alright now! The review has been posted successfully:

https://go-review.googlesource.com/#/c/9370/

Thanks for (week-end) support guys.

[gopherbot]

CL https://golang.org/cl/9370 mentions this issue.

[gravis]

@bradfitz I'm trying to update my changes, with a simpler version, but I can't push:

[...]
 ! [remote rejected] fix-leaking-parser -> fix-leaking-parser (prohibited by Gerrit)
error: failed to push some refs to 'https://go.googlesource.com/go'

Any idea to fix this? I can't find any thing in the guide related to this :(

Thanks,
Philippe

[bradfitz]

Let's move Gerrit support to a mailing list.

[gravis]

Sorry :)

[gravis]

Ok, I figured out, it was indeed in the doc, under the Revise and upload section.
I had to rebase commits, but I finally manage to push a new patch.

[gopherbot]

CL https://golang.org/cl/9501 mentions this issue.

[gopherbot]

CL https://golang.org/cl/9658 mentions this issue.

[gopherbot]

CL https://golang.org/cl/9841 mentions this issue.