From 370cadd0e43ddfcfd494eefefc5bd4e85d7efcf8 Mon Sep 17 00:00:00 2001
From: hopehook <hopehook.com@gmail.com>
Date: Fri, 8 Apr 2022 13:37:40 +0800
Subject: [PATCH] cmd/compile: add a test case and some comments for deadlock
 on syntax 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>
---
 src/cmd/compile/internal/noder/noder.go |  1 +
 test/fixedbugs/issue52127.go            | 62 +++++++++++++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 test/fixedbugs/issue52127.go

diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go
index bbd73aa8be..cc5610acda 100644
--- a/src/cmd/compile/internal/noder/noder.go
+++ b/src/cmd/compile/internal/noder/noder.go
@@ -40,6 +40,7 @@ func LoadPackage(filenames []string) {
 		noders[i] = &p
 	}
 
+	// Move the entire syntax processing logic into a separate goroutine to avoid blocking on the "sem".
 	go func() {
 		for i, filename := range filenames {
 			filename := filename
diff --git a/test/fixedbugs/issue52127.go b/test/fixedbugs/issue52127.go
new file mode 100644
index 0000000000..7738c3fabf
--- /dev/null
+++ b/test/fixedbugs/issue52127.go
@@ -0,0 +1,62 @@
+// run
+//go:build !js
+// +build !js
+
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Issue 52127: Too many syntax errors in many files can
+// cause deadlocks instead of displaying error messages
+// correctly.
+
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+)
+
+func main() {
+	dir, err := os.MkdirTemp("", "issue52127")
+	if err != nil {
+		panic(err)
+	}
+	defer os.RemoveAll(dir)
+
+	args := []string{"go", "build"}
+	write := func(prefix string, i int, data string) {
+		filename := filepath.Join(dir, fmt.Sprintf("%s%d.go", prefix, i))
+		if err := os.WriteFile(filename, []byte(data), 0o644); err != nil {
+			panic(err)
+		}
+		args = append(args, filename)
+	}
+
+	for i := 0; i < 100; i++ {
+		write("a", i, `package p
+`)
+	}
+	for i := 0; i < 100; i++ {
+		write("b", i, `package p
+var
+var
+var
+var
+var
+`)
+	}
+
+	cmd := exec.Command(args[0], args[1:]...)
+	output, err := cmd.CombinedOutput()
+	if err == nil {
+		panic("compile succeeded unexpectedly")
+	}
+	if !bytes.Contains(output, []byte("syntax error:")) {
+		panic(fmt.Sprintf(`missing "syntax error" in compiler output; got: 
+%s`, output))
+	}
+}
\ No newline at end of file