Rlimit cache rework for Go 1.23+

[kolyshkin]

Go 1.23 tightens access to internal symbols, and even puts runc into
"hall of shame" for using an internal symbol (recently added by commit
da68c8e). So, while not impossible, it becomes harder to access those
internal symbols, and it is a bad idea in general.

Since Go 1.23 includes https://go.dev/cl/588076, we can clean the
internal rlimit cache by setting the RLIMIT_NOFILE for ourselves,
essentially disabling the rlimit cache.

Once Go 1.22 is no longer supported, we will remove the go:linkname hack.

[kolyshkin]

This will fail CI because https://go.dev/cl/588076 is not yet in released Go versions.

[thaJeztah]

Thanks for working on this! <3

[lifubang]

NOTE this relies on golang.org/x/sys/unix having https://go.dev/cl/476695,
and Go having https://go.dev/cl/588076.

If we need golang to improve this issue, why not choose a simple way which can reduce two unnecessary syscalls.

golang/go#66797 (comment)
The core reason is that using Get/Set to clear the internal cache is not a atomic operation, I suggest to add a public method to clear this cache directly. For example: runtime.ClearSyscallRlimitCache().

[kolyshkin]

The core reason is that using Get/Set to clear the internal cache is not a atomic operation, I suggest to add a public method to clear this cache directly. For example: runtime.ClearSyscallRlimitCache().

We only do Set here (will update PR in a sec).

[lifubang]

We only do Set here

golang/go#66797 (comment)

[lifubang]

a853a82
Because we have used procReady msg to avoid the race, if we want to remove go linkname, I recommend to do the original get/set to clear the cache. But I strongly suggest golang to add a specific public API to do this.

[kolyshkin]

TODO: add CI for gotip, similar to how it's implemented here:
https://github.com/google/pprof/blob/main/.github/workflows/ci.yaml

Will do later.

[kolyshkin]

Codespell CI failure is unrelated; being fixed in #4291

[kolyshkin]

OK, this PR now has an infra in place to test https://go.dev/cl/588076 once it will be merged.

Currently, in Go 1.23 we have failures like this one (because the rlimit cache is not cleaned):

=== RUN   TestExecInUsernsRlimit
    execin_test.go:129: expected rlimit to be 1026, got 512
--- FAIL: TestExecInUsernsRlimit (0.18s)

Marking as draft for now, pending https://go.dev/cl/588076 merge.

[kolyshkin]

Now https://go.dev/cl/588076 is merged and everything works as it should, incl. go 1.23.

[thaJeztah]

Does that mean this is ready for review? 😅 (I see it's still in draft)

[kolyshkin]

Does that mean this is ready for review? 😅 (I see it's still in draft)

It is now; PTAL.

[rata]

LGTM, thanks!

[lifubang]

So we have to alway test the unreleased go version in the future? If there was something wrong in master branch of go repository, we would get ci failure too.

[kolyshkin]

So we have to alway test the unreleased go version in the future?

No, we don't.

The reason for adding this is simple. The code added is for Go 1.23, and there's no other way to test it, and I don't want to push the code that's not tested.

I can remove the gotip commit now when we saw it's working.

[kolyshkin]

Removed; PTAL @lifubang

[kolyshkin]

Let's merge this so we can move forward with https://go-review.googlesource.com/c/go/+/587918

[kolyshkin]

(and with the backport)