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

Jump to bottom

syscall: TestRlimit fails on darwin amd64 #40564

Closed
choleraehyq opened this issue Aug 4, 2020 · 9 comments
Closed

syscall: TestRlimit fails on darwin amd64 #40564

choleraehyq opened this issue Aug 4, 2020 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Milestone
Backlog

Comments

@choleraehyq
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version devel +f92337422e Sun Aug 2 00:04:25 2020 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/cholerae/Library/Caches/go-build"
GOENV="/Users/cholerae/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/cholerae/Documents/gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/cholerae/Documents/gopath"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/Users/cholerae/Documents/gopath/go"
GOSUMDB="sum.golang.google.cn"
GOTMPDIR=""
GOTOOLDIR="/Users/cholerae/Documents/gopath/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/cholerae/Documents/gopath/gotips/src/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lg/ld5t5rss459241qtzmqfp0h80000gn/T/go-build882775733=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

GOROOT_BOOTSTRAP=~/Documents/gopath/go1.14.6 ./all.bash

What did you expect to see?

All test passed.

What did you see instead?

--- FAIL: TestRlimit (0.00s)
    syscall_unix_test.go:347: Setrlimit: set failed: syscall.Rlimit{Cur:0x2800, Max:0x7fffffffffffffff} invalid argument
FAIL
FAIL	syscall	1.211s
@choleraehyq
Copy link
Contributor Author

According to man page,

The specified limit is invalid (e.g., RLIM_INFINITY or lower than rlim_cur).

RLIM_INFINITY is 0x7fffffffffffffff. Seems we should check whether set.Max is >= 0x7fffffffffffffff. Will send a CL.

@choleraehyq
Copy link
Contributor Author

It's a little bit weird. I found that there's nothing about set.Max, but set.Cur. After I changed set.Cur to 4096, the test passed. Seems rlim_cur must be equal or lower than 4096, but I can't find any document about it. Does anyone know the reason?
A C++ reproducer:

#include <sys/types.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <cerrno>
#include <iostream>

int main() {
  std::cout << OPEN_MAX << " " << RLIM_INFINITY << std::endl;
  struct rlimit r;
  r.rlim_cur = 4097;
  r.rlim_max = 10000;
  int ret;
  ret = setrlimit(RLIMIT_NOFILE, &r);
  std::cout << ret << std::endl;
  std::cout << std::strerror(errno) << std::endl;
}

will get

10240 9223372036854775807
-1
Invalid argument

but if I change r.rlim_cur to 4096, everything is fine.

@tklauser
Copy link
Member

tklauser commented Aug 4, 2020

On what version of macOS are you seeing this? Does this happen only sporadically (since you mentioned the test is flaky) or does it reproduce consistently?

FWIW, I can't reproduce this here on macOS Catalina (10.15.6).

@tklauser tklauser added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. OS-Darwin labels Aug 4, 2020
@choleraehyq
Copy link
Contributor Author

@tklauser I can reproduce this consistently, the title is not accurate, I'll change it.
I'm using macOS Catalina 10.15.5.

@choleraehyq choleraehyq changed the title syscall: TestRlimit flaky on darwin amd64 syscall: TestRlimit fails on darwin amd64 Aug 4, 2020
@tklauser tklauser removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 4, 2020
@tklauser
Copy link
Member

tklauser commented Aug 4, 2020

@choleraehyq Can you try getrlimit(RLIMIT_NOFILE, ...) in the C++ reproducer before setrlimit and report the limits it returns? Could you also post the output of launchctl limit maxfiles and sysctl -a | grep maxfiles?

@choleraehyq
Copy link
Contributor Author

@tklauser

➜  test launchctl limit maxfiles
	maxfiles    4096           10240
➜  test sysctl -a | grep maxfiles
kern.maxfiles: 10240
kern.maxfilesperproc: 4096
➜  test cat rlimit.cc
#include <sys/types.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <cerrno>
#include <iostream>

int main() {
  // std::cout << OPEN_MAX << " " << RLIM_INFINITY << std::endl;
  struct rlimit r;
  int ret;
  ret = getrlimit(RLIMIT_NOFILE, &r);
  std::cout << "getrlimit" << std::endl;
  std::cout << r.rlim_cur << " " << r.rlim_max << std::endl;
  std::cout << ret << std::endl;
  std::cout << std::strerror(errno) << std::endl;
  std::cout << "setrlimit" << std::endl;
  r.rlim_cur = 4097;
  r.rlim_max = 10000;
  ret = setrlimit(RLIMIT_NOFILE, &r);
  std::cout << ret << std::endl;
  std::cout << std::strerror(errno) << std::endl;
}
➜  test clang++ rlimit.cc -o rlimit
➜  test ./rlimit
getrlimit
256 9223372036854775807
0
Undefined error: 0
setrlimit
-1
Invalid argument

@tklauser
Copy link
Member

tklauser commented Aug 4, 2020

@choleraehyq thanks. Not sure why you're seeing these values, but the sysctl values probably explain the limits. Not sure why it is different from my limits though:

% launchctl limit maxfiles
	maxfiles    256            unlimited
% sysctl -a | grep maxfiles
kern.maxfiles: 49152
kern.maxfilesperproc: 24576
% clang++ rlimit.cc -o rlimit
% ./rlimit
getrlimit
256 9223372036854775807
0
Undefined error: 0
setrlimit
0
Undefined error: 0

Maybe we should check the value of kern.maxfilesperproc in TestRlimit? But given that it just tests the basic functionality of Getrlimit/Setrlimit we might also just decrease set.Cur to 4096.

/cc @ianlancetaylor @randall77

@gopherbot
Copy link

Change https://golang.org/cl/246658 mentions this issue: syscall: check kern.maxfilesperproc in TestRlimit on darwin

@toothrot toothrot added this to the Backlog milestone Aug 5, 2020
@gopherbot
Copy link

Change https://golang.org/cl/250000 mentions this issue: unix: cap RLIMIT_NOFILE soft limit in TestRlimit on darwin

gopherbot pushed a commit to golang/sys that referenced this issue Aug 24, 2020
@tklauser
unix: cap RLIMIT_NOFILE soft limit in TestRlimit on darwin
c12d262 
On some machines, kern.maxfilesperproc is 4096. If Rlimit.Cur is larger
than that, Setrlimit will get an errEINVAL.

Same as CL 246658 did in package syscall.

Updates golang/go#40564

Change-Id: I8c45a23352fa2039772e04205680640e8a0e1840
Reviewed-on: https://go-review.googlesource.com/c/sys/+/250000
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>
@golang golang locked and limited conversation to collaborators Aug 24, 2021
gopherbot pushed a commit that referenced this issue Mar 4, 2022
@ianlancetaylor
syscall: remove TestRlimit
1e122e3 
It's more trouble than it's worth. New code should be using x/sys/unix
anyhow.

Fixes #40564
Fixes #51479

Change-Id: I1c0e13f494380c1565e98359f088af9f52790b79
Reviewed-on: https://go-review.googlesource.com/c/go/+/390020
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Mar 7, 2022
@ianlancetaylor @dmitshur
[release-branch.go1.18] syscall: remove TestRlimit
520f0d0 
It's more trouble than it's worth. New code should be using x/sys/unix
anyhow.

Fixes #40564
Fixes #51479

Change-Id: I1c0e13f494380c1565e98359f088af9f52790b79
Reviewed-on: https://go-review.googlesource.com/c/go/+/390020
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 1e122e3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/390022
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@toothrot @tklauser @gopherbot @choleraehyq