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

x/sys/unix: TestClockNanosleep failures on linux-mipsle-rtrk builder #35622

Closed
bcmills opened this issue Nov 15, 2019 · 5 comments
Closed

x/sys/unix: TestClockNanosleep failures on linux-mipsle-rtrk builder #35622

bcmills opened this issue Nov 15, 2019 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.14

Comments

@bcmills
Copy link
Contributor

bcmills commented Nov 15, 2019 •
edited
Loading

https://build.golang.org/log/f0d29a593ccba5704246987f49074bfca43dd1b9

--- FAIL: TestClockNanosleep (0.10s)
    syscall_linux_test.go:580: ClockNanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &unix.Timespec{Sec:1573834610, Nsec:328568608} (=2019-11-15 17:16:50.328568608 +0100 CET m=+0.544561463), nil) = interrupted system call
FAIL
FAIL	golang.org/x/sys/unix	0.599s

CC @tklauser @bogojevic @milanknezevic @cherrymui
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 15, 2019
@bcmills bcmills added this to the Go1.14 milestone Nov 15, 2019
@ianlancetaylor
Copy link
Member

Now that we use signals for preemption I guess all those calls need to check for EINTR.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/207285 mentions this issue: unix: don't fail TestClockNanosleep on EINTR from ClockNanosleep

@bcmills
Copy link
Contributor Author

bcmills commented Nov 16, 2019

@ianlancetaylor, aren't the runtime's handlers all registered with SA_RESTART set?

I would expect preemption to produce spurious EINTR return values in some cgo programs due to #20400, but I don't understand why it would affect a pure-Go package test.

@ianlancetaylor
Copy link
Member

SA_RESTART does not apply to all system calls. For the Linux kernel, see http://man7.org/linux/man-pages/man7/signal.7.html, where it says that the clock_nanosleep system call, among others, always returns EINTR even if the signal was registered with SA_RESTART.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/207290 mentions this issue: unix: fix EINTR check in TestClockNanosleep

gopherbot pushed a commit to golang/sys that referenced this issue Nov 18, 2019
@tklauser @bradfitz
unix: fix EINTR check in TestClockNanosleep
6254a7c 
err == EINTR needs to be checked before err != nil for the interrupted
syscall to be retried properly.

Follow-up for CL 207285

Updates golang/go#35622

Change-Id: I0f8c1a75eb96e11aaba284eb75716db044257cbd
Reviewed-on: https://go-review.googlesource.com/c/sys/+/207290
Run-TryBot: Tobias Klauser <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
@golang golang locked and limited conversation to collaborators Nov 16, 2020
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.
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
3 participants
@ianlancetaylor @bcmills @gopherbot