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: document concurrency properties for the instances returned by (*Template).New #30281

Closed
bep opened this issue Feb 17, 2019 · 2 comments

Comments

Projects
None yet
4 participants
@bep
Copy link
Contributor

commented Feb 17, 2019

package main

import (
	"fmt"
	"sync"
	"text/template"
)

func main() {
	var wg sync.WaitGroup

	tpl := template.New("")

	for i := 1; i < 20; i++ {
		name := fmt.Sprintf("n%d.txt", i)
		wg.Add(1)
		go func() {
			defer wg.Done()
			tpl.New(name).Parse("Go!")
		}()
	}

	wg.Wait()
}

The above fails with:

fatal error: concurrent map read and map write

goroutine 36 [running]:
runtime.throw(0x1136d33, 0x21)
	/Users/bep/sdk/go1.12beta2/src/runtime/panic.go:617 +0x72 fp=0xc000075d78 sp=0xc000075d48 pc=0x1029d02
runtime.mapaccess1_faststr(0x110d820, 0xc000066210, 0xc000016230, 0x7, 0x0)
	/Users/bep/sdk/go1.12beta2/src/runtime/map_faststr.go:21 +0x464 fp=0xc000075de8 sp=0xc000075d78 pc=0x1010a34
text/template.(*Template).associate(0xc00004a340, 0xc00004a340, 0xc0000ae800, 0xc00006ec01)
	/Users/bep/sdk/go1.12beta2/src/text/template/template.go:217 +0x62 fp=0xc000075e28 sp=0xc000075de8 pc=0x10ec762
text/template.(*Template).AddParseTree(0xc00004a340, 0xc000016230, 0x7, 0xc0000ae800, 0x0, 0x0, 0x0)
	/Users/bep/sdk/go1.12beta2/src/text/template/template.go:128 +0xfa fp=0xc000075e78 sp=0xc000075e28 pc=0x10ebb0a
text/template.(*Template).Parse(0xc00004a340, 0x1131652, 0x3, 0x0, 0x0, 0x0)
	/Users/bep/sdk/go1.12beta2/src/text/template/template.go:203 +0x1f8 fp=0xc000075f78 sp=0xc000075e78 pc=0x10ec528
main.main.func1(0xc000016114, 0xc00004a0c0, 0xc000016230, 0x7)
	/Users/bep/dev/go/bep/temp/main.go:19 +0xfa fp=0xc000075fc0 sp=0xc000075f78 pc=0x10ee17a
runtime.goexit()
	/Users/bep/sdk/go1.12beta2/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc000075fc8 sp=0xc000075fc0 pc=0x1052f41
created by main.main
	/Users/bep/dev/go/bep/temp/main.go:17 +0x13b

goroutine 1 [semacquire]:
sync.runtime_Semacquire(0xc000016114)
	/Users/bep/sdk/go1.12beta2/src/runtime/sema.go:56 +0x39
sync.(*WaitGroup).Wait(0xc000016114)
	/Users/bep/sdk/go1.12beta2/src/sync/waitgroup.go:130 +0x65
main.main()
	/Users/bep/dev/go/bep/temp/main.go:23 +0x161
Error: process exited with code 2.

I'm currently running go1.12beta2 (yes, I know), but I have had this failure on earlier versions, too.

Note that I'm not saying that this is a bug (the doc for Parse isn't clear on this), but as this has hit me more than once in real programs, I think at least a documentation update is needed.

@bep bep changed the title text/template: concurrent map writes in Parse text/template: concurrent map read and map write in Parse Feb 17, 2019

@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 17, 2019

See this comment: #28461 (comment)

Also see the pieces of documentation, like:

Once parsed, a template may be executed safely in parallel, although if parallel executions share a Writer the output may be interleaved.

Nothing seems to point to me that Parse could be safe for concurrent use within one template. However, the New method does say that it allocates a new template, so one could assume the two could be parsed concurrently.

Looking at the implementation, it looks like everything is shared between templates in the same "group", minus the name, the delimiters, and the parsed template. So I don't think we can fix the code without introducing large refactors and changes to the design.

I think we can just clarify in the Template.New method that the new template shares data with the parent, so they are still the same template when it comes to whether concurrent method calls are allowed.

@mvdan mvdan self-assigned this Feb 17, 2019

@mvdan mvdan added this to the Go1.13 milestone Feb 17, 2019

@bcmills bcmills changed the title text/template: concurrent map read and map write in Parse text/template: document concurrency properties for the instances returned by (*Template).New Feb 28, 2019

@bcmills bcmills added the NeedsFix label Feb 28, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Apr 16, 2019

Change https://golang.org/cl/172297 mentions this issue: text/template: clarify the safety of Template.New

@gopherbot gopherbot closed this in eb2fabf Jun 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.