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

syscall: should not clear nofile rlimit cache when setting other process's nofile rlimit #67184

Closed
lifubang opened this issue May 5, 2024 · 3 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@lifubang
Copy link
Contributor

lifubang commented May 5, 2024

Go version

go version go1.19

Output of go env in your module/workspace:

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/acmcoder/.cache/go-build"
GOENV="/home/acmcoder/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/acmcoder/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/acmcoder/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.13"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build6687487=/tmp/go-build -gno-record-gcc-switches"

What did you do?

When we use unix.Prlimit to set other process's nofile limit, we should not clear the calling process's nofile limit cache, it will cause the nofile limit setting invalid when we use unix.Exec.
The reprodecing code:

package main

import (
	"fmt"
	"os"
	"os/exec"

	"golang.org/x/sys/unix"
)

func main() {
	// Usage: ./prlimit true or ./prlimit false
	ifPrlimit := os.Args[1]
	if ifPrlimit == "true" {
		fmt.Println("Prlimit is true")
	} else {
		fmt.Println("Prlimit is false")
	}

	// Start a child process firstly,
	// so we can use Prlimit to set it's nofile rlimit.
	cmd := exec.Command("sleep", "infinity")
	cmd.Start()

	// To print out the process's nofile limit
	var lim unix.Rlimit
	if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &lim); err == nil {
		fmt.Printf("(1) init curr: %d, max: %d\n", lim.Cur, lim.Max)
	}

	// If we run ./prlimit true, we use unix.Prlimit to set child process's nofile limit,
	// then it will cause a bug for calling process.
	if ifPrlimit == "true" {
		if err := unix.Prlimit(cmd.Process.Pid, unix.RLIMIT_NOFILE, &lim, nil); err != nil {
			fmt.Println("Prlimit failed", err)
		}
		cmd.Process.Kill()
		cmd.Process.Wait()
		if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &lim); err == nil {
			fmt.Printf("(2) after Prlimit other process, curr: %d, max: %d\n", lim.Cur, lim.Max)
		}
	}

	// The above mentiond bug is here.
	unix.Exec("/bin/sh", []string{"/bin/sh", "-c", "ulimit -n"}, os.Environ())
}

What did you see happen?

acmcoder@acmcoder:~/rlimits$ # Execve a process without Prlimit
acmcoder@acmcoder:~/rlimits$ ./prlimit false
Prlimit is false
(1) init curr: 1048576, max: 1048576
1024

acmcoder@acmcoder:~/rlimits$ # Execve a process after Prlimit
acmcoder@acmcoder:~/rlimits$ ./prlimit true
Prlimit is true
(1) init curr: 1048576, max: 1048576
(2) after Prlimit other process, curr: 1048576, max: 1048576
1048576

What did you expect to see?

The last output of ./prlimit true should be the same as ./prlimit false.

lifubang added a commit to lifubang/go that referenced this issue May 5, 2024
Fixes: golang#67184
The syscall unix.Prlimit can set other process's nofile limit, if we do
that, we should not clear the calling process's nofile rlimit cache.

Signed-off-by: lifubang <lifubang@acmcoder.com>
@seankhliao seankhliao changed the title syscall.prlimit: We should not clear nofile rlimit cache when setting other process's nofile rlimit syscall: should not clear nofile rlimit cache when setting other process's nofile rlimit May 5, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 5, 2024
lifubang added a commit to lifubang/go that referenced this issue May 6, 2024
Fixes: golang#67184

Signed-off-by: lifubang <lifubang@acmcoder.com>
@cherrymui
Copy link
Member

This sounds like a dup of #66797. Closing as a dup. Feel free to reopen if I'm mistaken. Thanks.

@cherrymui cherrymui closed this as not planned Won't fix, can't repro, duplicate, stale May 6, 2024
@ianlancetaylor
Copy link
Member

Actually, this one is different. #66797 is about some other process changing the rlimit of a Go process. This issue is about a Go process changing the rlimit of some other process. In the latter case we are accidentally thinking that the Go process is changing its own rlimit.

@ianlancetaylor ianlancetaylor reopened this May 6, 2024
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 6, 2024
@cherrymui cherrymui added this to the Backlog milestone May 6, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/583299 mentions this issue: syscall: don't change local limit if prlimit used for another process

@mknyszek mknyszek moved this to In Progress in Go Compiler / Runtime May 8, 2024
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 13, 2024
@dmitshur dmitshur modified the milestones: Backlog, Go1.23 May 13, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

Successfully merging a pull request may close this issue.

5 participants