cmd/compile: deadlock on syntax error #52127
Comments
tdegris
changed the title
ianlancetaylor
changed the title
ianlancetaylor
added
the
NeedsFix
|
Change https://go.dev/cl/398014 mentions this issue: |
|
I wrote a demo that reproduces the problem. package main
import (
"fmt"
)
const (
FileNames = 5
OpenFileLimit = 1 // if BufferCount == 0 and OpenFileLimit < FileNames, deadlock may occur
BufferCount = 0 // if BufferCount > 0, there is no deadlock for range p.err
)
type noder struct {
err chan string
}
func main() {
sem := make(chan struct{}, OpenFileLimit)
noders := make([]*noder, FileNames)
i := 0
for i < FileNames {
p := noder{
err: make(chan string, BufferCount),
}
noders[i] = &p
go func(c int) {
sem <- struct{}{}
defer func() { <-sem }()
defer close(p.err)
text := fmt.Sprintf("go func: %d finished", c)
p.err <- text
fmt.Println(text)
}(i)
i = i + 1
}
for _, p := range noders {
for e := range p.err {
fmt.Println("received:", e)
}
}
} |
|
Thanks. I don't think changing the error channel to buffered fixes the issue though. It'll just push the deadlock to only affect when there are files with multiple syntax errors. I think instead we need to prioritize that the semaphore is use to parse the first N files, rather than just any N. I think this can be fixed by acquiring the semaphore in the loop outside of the goroutine. But then the loop itself needs to be on a separate goroutine too. |
Thanks. |
|
I wrote another demo to show my understanding: package main
import (
"context"
"fmt"
"golang.org/x/sync/semaphore"
)
const (
FileNames = 5
OpenFileLimit = 1
BufferCount = 1 // if BufferCount > 0, there is no deadlock for range p.err
)
type noder struct {
err chan string
}
func main() {
ctx := context.TODO()
sem := semaphore.NewWeighted(int64(OpenFileLimit))
noders := make([]*noder, FileNames)
for i := range noders {
if err := sem.Acquire(ctx, 1); err != nil {
fmt.Printf("Failed to acquire semaphore: %v", err)
break
}
go func(c int) {
defer sem.Release(1)
p := noder{
err: make(chan string, BufferCount),
}
noders[c] = &p
defer close(p.err)
text := fmt.Sprintf("The goroutine func: %d finished", c)
p.err <- text
fmt.Println(text)
}(i)
}
if err := sem.Acquire(ctx, int64(OpenFileLimit)); err != nil {
fmt.Printf("Failed to acquire semaphore: %v", err)
}
for _, p := range noders {
for e := range p.err {
fmt.Println("Received:", e)
}
}
}Or package main
import (
"fmt"
"sync"
)
const (
FileNames = 5
OpenFileLimit = 1
BufferCount = 1 // if BufferCount > 0, there is no deadlock for range p.err
)
type noder struct {
err chan string
}
func main() {
var wg sync.WaitGroup
sem := make(chan struct{}, OpenFileLimit)
noders := make([]*noder, FileNames)
for i := range noders {
wg.Add(1)
go func(c int) {
sem <- struct{}{}
defer func() {
<-sem
wg.Done()
}()
p := noder{
err: make(chan string, BufferCount),
}
noders[c] = &p
defer close(p.err)
text := fmt.Sprintf("The goroutine func: %d finished", c)
p.err <- text
fmt.Println(text)
}(i)
}
wg.Wait()
for _, p := range noders {
for e := range p.err {
fmt.Println("Received:", e)
}
}
} |
|
Here's the code I had in mind: https://go.dev/play/p/yKyrv7NiVB9 |
Thanks, I hadn't thought of that. |
|
Change https://go.dev/cl/399054 mentions this issue: |
… error After CL 398014 fixed a compiler deadlock on syntax errors, this CL adds a test case and more details for that. How it was fixed: CL 57751 introduced a channel "sem" to limit the number of simultaneously open files. Unfortunately, when the number of syntax processing goroutines exceeds this limit, will easily trigger deadlock. In the original implementation, "sem" only limited the number of open files, not the number of concurrent goroutines, which will cause extra goroutines to block on "sem". When the p.err of the following iteration happens to be held by the blocking goroutine, it will fall into a circular wait, which is a deadlock. CL 398014 fixed the above deadlock, also see issue #52127. First, move "sem <- struct{}{}" to the outside of the syntax processing goroutine, so that the number of concurrent goroutines does not exceed the number of open files, to ensure that all goroutines in execution can eventually write to p.err. Second, move the entire syntax processing logic into a separate goroutine to avoid blocking on the producer side. Change-Id: I1bb89bfee3d2703784f0c0d4ded82baab2ae867a Reviewed-on: https://go-review.googlesource.com/c/go/+/399054 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
Compiling
github.com/tdegris/base16-gocrashes the compiler. Instructions to reproduce the bug:What did you expect to see?
A compile error or a successful compilation. Sometimes, the compilation does return the following (correct) syntax errors:
What did you see instead?
See the full stack trace.
The text was updated successfully, but these errors were encountered: