Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text/template: data race in template parsing #53234

Closed
eliben opened this issue Jun 4, 2022 · 2 comments
Closed

text/template: data race in template parsing #53234

eliben opened this issue Jun 4, 2022 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@eliben
Copy link
Member

eliben commented Jun 4, 2022

What version of Go are you using (go version)?

tip

Issue description

Uncovered while working on https://go-review.googlesource.com/c/go/+/410296

Tree.Parse in text/template/parse/parse.go invokes:

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

Where startParse is:

func (t *Tree) startParse(funcs []map[string]any, lex *lexer, treeSet map[string]*Tree) {
	t.Root = nil
	t.lex = lex
	t.vars = []string{"$"}
	t.funcs = funcs
	t.treeSet = treeSet
	lex.breakOK = !t.hasFunction("break")
	lex.continueOK = !t.hasFunction("continue")
}

The lexer passed into startParse starts a goroutine which runs and accesses breakOK and continueOK, and potentially races with their initialization in startParse.

Before https://go-review.googlesource.com/c/go/+/410296 this issue doesn't manifest because the lexer's items channel is blocking and the lexer won't get to an identifier until the reader pulled at least one token. Once a buffer is added to the channel, however, the lexer runs ahead a bit and triggers the race.

The fix should be to remove the racing access to these fields.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/410414 mentions this issue: text/template/parse: fix data race on lexer initialization

@eliben
Copy link
Member Author

eliben commented Jun 4, 2022

https://go-review.googlesource.com/c/go/+/410414 is one potential solution to the data race.

The downside is that it makes the initialization of lex(...) a bit verbose, with three boolean parameters. Some alternatives considered:

  • Create a "config" struct for lex(...) with named fields, making this intialization more readable
  • Separate the goroutine launch of the lexer from the lexer's creation: something like a separate start method. I don't really like this as it complicates the API and still leaves races lurking if users do things in the wrong order.

@eliben eliben self-assigned this Jun 4, 2022
@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 5, 2022
@dmitshur dmitshur added this to the Go1.19 milestone Jun 6, 2022
@rsc rsc unassigned eliben Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants