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

internal/poll: fcntl() should loop on EINTR #41115

Closed
networkimprov opened this issue Aug 28, 2020 · 4 comments
Closed

internal/poll: fcntl() should loop on EINTR #41115

networkimprov opened this issue Aug 28, 2020 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@networkimprov
Copy link

networkimprov commented Aug 28, 2020

The internal/poll.fcntl sites need an EINTR loop. The docs list EINTR for some features, and darwin F_FULLFSYNC hits the disk.
https://man7.org/linux/man-pages/man2/fcntl.2.html

https://github.com/golang/go/blob/master/src/internal/poll/fcntl_libc.go
https://github.com/golang/go/blob/master/src/internal/poll/fcntl_syscall.go
https://github.com/golang/go/blob/master/src/internal/poll/fd_fsync_darwin.go
possibly others...

It might be worth a comment on poll.CloseFunc, to note that errors don't mean failure, so retry is incorrect:
https://man7.org/linux/man-pages/man2/close.2.html

Also, seems like a backport fix...

@gopherbot add NeedsFix
cc @ianlancetaylor

@gopherbot gopherbot added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 28, 2020
@ianlancetaylor
Copy link
Contributor

Note that we never call fcntl with the operations that the GNU/Linux man page says can return EINTR.

I guess we do need a loop on Darwin, though.

@ianlancetaylor ianlancetaylor added this to the Backlog milestone Aug 28, 2020
@ianlancetaylor
Copy link
Contributor

We aren't going to backport any changes in this area, any EINTR errors are not a regression.

@networkimprov
Copy link
Author

networkimprov commented Aug 28, 2020

I'd worry that some other fcntl() op might return EINTR -- if not now, later.

Could we tag for 1.16?

@gopherbot
Copy link

Change https://golang.org/cl/258542 mentions this issue: internal/poll: use ignoringEINTR in Darwin Fsync

@golang golang locked and limited conversation to collaborators Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants