Skip to content

Commit aa1b50e

Browse files
mknyszekgopherbot
authored andcommitted
runtime: make tidExists more robust
The LockThreadExit tests in the runtime have been observed to fail after reading /proc/self/task/<tid>/stat and blindly assuming its contents followed a specific format. The parsing code is also wrong, because splitting by spaces doesn't work when the comm name contains a space. It also ignores errors without reporting them, which isn't great. This change rewrites tidExists to be more robust by using /proc/self/task/<tid>/status instead. It also modifies tidExists' signature to report an error to its caller. Its caller then prints that error. Ignoring a non-not-exist error with opening this file is the likely but unconfirmed cause of #65736 (ESRCH). This change also checks for that error explicitly as an optimistic fix. Fixes #65736. Change-Id: Iea560b457d514426da2781b7eb7b8616a91ec23b Reviewed-on: https://go-review.googlesource.com/c/go/+/567938 Auto-Submit: Michael Knyszek <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]>
1 parent 802473c commit aa1b50e

File tree

3 files changed

+43
-9
lines changed

3 files changed

+43
-9
lines changed

src/runtime/testdata/testprog/lockosthread.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,11 @@ func LockOSThreadAlt() {
9090
println("locked thread reused")
9191
os.Exit(1)
9292
}
93-
exists, supported := tidExists(subTID)
93+
exists, supported, err := tidExists(subTID)
94+
if err != nil {
95+
println("error:", err.Error())
96+
return
97+
}
9498
if !supported || !exists {
9599
goto ok
96100
}

src/runtime/testdata/testprog/syscalls_linux.go

+36-6
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"bytes"
99
"fmt"
1010
"internal/testenv"
11+
"io"
1112
"os"
1213
"syscall"
1314
)
@@ -16,14 +17,43 @@ func gettid() int {
1617
return syscall.Gettid()
1718
}
1819

19-
func tidExists(tid int) (exists, supported bool) {
20-
stat, err := os.ReadFile(fmt.Sprintf("/proc/self/task/%d/stat", tid))
21-
if os.IsNotExist(err) {
22-
return false, true
20+
func tidExists(tid int) (exists, supported bool, err error) {
21+
// Open the magic proc status file for reading with the syscall package.
22+
// We want to identify certain valid errors very precisely.
23+
statusFile := fmt.Sprintf("/proc/self/task/%d/status", tid)
24+
fd, err := syscall.Open(statusFile, syscall.O_RDONLY, 0)
25+
if errno, ok := err.(syscall.Errno); ok {
26+
if errno == syscall.ENOENT || errno == syscall.ESRCH {
27+
return false, true, nil
28+
}
29+
}
30+
if err != nil {
31+
return false, false, err
32+
}
33+
status, err := io.ReadAll(os.NewFile(uintptr(fd), statusFile))
34+
if err != nil {
35+
return false, false, err
36+
}
37+
lines := bytes.Split(status, []byte{'\n'})
38+
// Find the State line.
39+
stateLineIdx := -1
40+
for i, line := range lines {
41+
if bytes.HasPrefix(line, []byte("State:")) {
42+
stateLineIdx = i
43+
break
44+
}
45+
}
46+
if stateLineIdx < 0 {
47+
// Malformed status file?
48+
return false, false, fmt.Errorf("unexpected status file format: %s:\n%s", statusFile, status)
49+
}
50+
stateLine := bytes.SplitN(lines[stateLineIdx], []byte{':'}, 2)
51+
if len(stateLine) != 2 {
52+
// Malformed status file?
53+
return false, false, fmt.Errorf("unexpected status file format: %s:\n%s", statusFile, status)
2354
}
2455
// Check if it's a zombie thread.
25-
state := bytes.Fields(stat)[2]
26-
return !(len(state) == 1 && state[0] == 'Z'), true
56+
return !bytes.Contains(stateLine[1], []byte{'Z'}), true, nil
2757
}
2858

2959
func getcwd() (string, error) {

src/runtime/testdata/testprog/syscalls_none.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ func gettid() int {
1111
return 0
1212
}
1313

14-
func tidExists(tid int) (exists, supported bool) {
15-
return false, false
14+
func tidExists(tid int) (exists, supported bool, err error) {
15+
return false, false, nil
1616
}
1717

1818
func getcwd() (string, error) {

0 commit comments

Comments
 (0)