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
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bep
Copy link
Contributor

bep 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
Copy link
Member

mvdan 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 added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Documentation labels Feb 28, 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 The path to resolution is known, but the work has not been done. label Feb 28, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 28, 2019
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Jun 6, 2020
@rsc rsc unassigned mvdan Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation 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