-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
html/template: concurrent map read and map write in execute #16101
Comments
To add some more context. I have a website that I built using Hugo. It has over 4500 pages. I started implementing the Unfortunately, I cannot make the repo for the site open to the public due to legal limitations. But I will be glad to assist with the investigation however I can. |
What do you mean by "taxonomies"? Without a small example that we can use to reproduce the issue, it's going to be hard to diagnose the problem. We're not going to be able to tackle this for the 1.7 release, as we are now too close to a release candidate. But since this was not a regression between 1.6 and 1.7, it's okay to defer it for 1.8. |
Can you please build a version of hugo using the race runtime and post the On Sat, Jun 18, 2016 at 9:20 PM, Christopher Mancini <
|
@adg @davecheney somehow the issue no longer occurred after I finished implementing Go template blocks across the entire site. No idea why I can't reproduce it anymore. |
OK, thanks for letting us know. |
@adg I opened this issue, and this is still an open issue in Hugo, and most signals point to a real issue in Go. That it is somehow "no longer an issue" for the original reporter does not equal "no longer an issue" (he moved his templates around, aka worked his way around the issue). There is a stack trace. |
@bep if there's an issue to be fixed in the template package, then we need to track it down. To do that we need a self-contained example that demonstrates the problem. |
We have a reproduce-able case in a private Hugo site repo, I will try to boil it down, but haven't had any success yet. But here is the trace from the race detector:
What I see above is:
|
@adg here is a repro that crashes consistently on my MacBook dual core / macOS Sierra / Go 1.7.1: package main
import (
"html/template"
"io/ioutil"
"log"
"sync"
"time"
)
var (
overlayTmpl *template.Template
masterTmpl *template.Template
)
func main() {
const (
master = `
<!DOCTYPE HTML>
<html>
<head>
<title>
{{block "a" . }}Default{{end}}
</title>
</head>
<body>
{{block "b" .}}Default{{end}}
{{block "c" .}}Default{{end}}
</body>
</html>
`
overlay = `
{{define "b"}}A{{end}}
`
)
var err error
masterTmpl, err = template.New("master").Parse(master)
if err != nil {
log.Fatal(err)
}
overlayTmpl, err = template.Must(masterTmpl.Clone()).Parse(overlay)
if err != nil {
log.Fatal(err)
}
var wg sync.WaitGroup
for i := 0; i < 100; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for i := 0; i < 1000; i++ {
if err := overlayTmpl.Execute(ioutil.Discard, "data"); err != nil {
log.Fatal(err)
}
time.Sleep(23 * time.Millisecond)
}
}()
}
wg.Wait()
log.Println("Done ...")
}
This is becoming a fairly painful issue in Hugo -- and any advice for a workaround would be appreciated. |
Nice. @bep go run main.go -race |
This is an html/template bug, not a text/template bug. Can someone change the title? The fix is simple, but the test is less so. I'll mail a CL shortly. |
If the "fix is simple" I suggest adding this to the 1.7.2 milestone: It renders the |
CL https://golang.org/cl/31092 mentions this issue. |
@bep, the bar for point release cherry picks is very high. If it's not a regression from Go 1.6 then it probably doesn't qualify. |
@bep here's a hacky workaround that should work in the meantime. masterTmpl, err = template.New("master").Parse(master)
if err != nil {
log.Fatal(err)
}
overlayTmpl, err = template.Must(masterTmpl.Clone()).Parse(overlay)
if err != nil {
log.Fatal(err)
}
overlayTmpl = overlayTmpl.Lookup(overlayTmpl.Name()) // horrible hack That fixes your repro, at least. |
There are no hard rules. Only a set of considerations which weigh into the decision. I'll let others decide on this one since I'm not the most familiar with this code. |
@cespare your suggested workaround works great for Hugo and myself (gohugoio/hugo@5882567). Thanks again. |
CL https://golang.org/cl/33210 mentions this issue. |
This change redoes the fix for #16101 (CL 31092) in a different way by making t.Clone return the template associated with the t.Name() while allowing for the case that a template of the same name is define-d. Fixes #17735. Change-Id: I1e69672390a4c81aa611046a209008ae4a3bb723 Reviewed-on: https://go-review.googlesource.com/33210 Run-TryBot: Caleb Spare <cespare@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
Go 1.6.2.
See gohugoio/hugo#2224
In an ideal world, I would have investigated further before posting this issue, but I'm short of time these days -- thought it would be better to just post it than to wait for an open time window.
I suspect this may be a artifact of the new
block
keyword in Go .1.6, which changed the semantics of when a template could be changed.The text was updated successfully, but these errors were encountered: