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.

Sign up for GitHub

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

Jump to bottom

syscall: caching RLIMIT_NOFILE is wrong implementation #66797

Open
ls-ggg opened this issue Apr 12, 2024 · 4 comments
Open

syscall: caching RLIMIT_NOFILE is wrong implementation #66797

ls-ggg opened this issue Apr 12, 2024 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@ls-ggg
Copy link

ls-ggg commented Apr 12, 2024 •
edited

Go version

go version go1.20.13 linux/amd64

Output of go env in your module/workspace:

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

What did you do?

RLIMIT_NOFILE is cached in the go runtime, and RLIMIT_NOFILE in the cache is restored for the child process before exec.

Assume that the system's default RLIMIT_NOFILE is 1024, and process A generates child process B through fork. B is a go application, so the go runtime will cache RLIMIT_NOFILE as 1024. When I set B's RLIMIT_NOFILE to 10000 through prlimit in A, and use exec.Command to create process C in B, C's RLIMIT_NOFILE is restored to 1024.

This behavior is incompatible with the operating system. It also caused problems with runc:
opencontainers/runc#4195

The problem originates from f5eef58

What did you see happen?

Reference opencontainers/runc#4195

What did you expect to see?

RLIMIT_NOFILE should be correctly inherited to child processes
@cagedmantis cagedmantis changed the title syscall：Caching RLIMIT_NOFILE is wrong implementation syscall: caching RLIMIT_NOFILE is wrong implementation Apr 12, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 12, 2024
@ianlancetaylor
Copy link
Contributor

Please don't show us code in images. Link to go.googlesource.com or GitHub instead. Images are difficult to read. Thanks.

@ianlancetaylor
Copy link
Contributor

If I understand this correctly, the problem is that if process A uses prlimit to change the resource limit in process B, then the Go standard library isn't aware of that when process B starts process C.

We can't change the current behavior in which process B passes the original RLIMIT_NOFILE value on to process C. That is likely to break existing programs that do not expect a large RLIMIT_NOFILE, as discussed in #46279. That issues shows real failure cases when we didn't do that, so we can't change back.

So I think the question is whether there is a way that process B could detect that the process limit was changed by some other process via prlimit. In that case we should pass down the updated value, just as we already do if process B explicitly calls syscall.Setrlimit itself.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2024
@cagedmantis cagedmantis added this to the Backlog milestone Apr 12, 2024
@cagedmantis
Copy link
Contributor

@golang/runtime

@ls-ggg
Copy link
Author

ls-ggg commented Apr 15, 2024

Please don't show us code in images. Link to go.googlesource.com or GitHub instead. Images are difficult to read. Thanks.
Ok, I've replaced the image with a commit link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Backlog
Development

No branches or pull requests
5 participants
@cagedmantis @mknyszek @ianlancetaylor @gopherbot @ls-ggg