-
Notifications
You must be signed in to change notification settings - Fork 18k
internal/poll: can get EINTR reading from CIFS #38836
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
Comments
I believe this is expected due to runtime changes in Go 1.14, see: https://golang.org/doc/go1.14#runtime. |
Maybe. Seems like a fairly simple solution. If there is confirmation/quorum
on this being the issue, I can create a PR (if it's not so simple that
someone else doesn't just jump on it immediately).
…On Sun, May 3, 2020, 19:00 Matt Layher ***@***.***> wrote:
I believe this is expected due to runtime changes in Go 1.14, see:
https://golang.org/doc/go1.14#runtime.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#38836 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFOW2RQOQN5R7QSRMLO7OLRPXZRLANCNFSM4MYGYLUQ>
.
|
No, 1.14 triggers more signals, but Go installs signal handlers with Is this a pure Go program? Can you run the program under |
Yes, it's definitely pure Go (a principal feature of the project). Okay. I'm running it with |
Got a couple. 1: Triggered in lines 423-427
2: Triggered in lines 475-481
|
Thanks. I see that the |
Yeah. CIFS. Due to unusual preemption? Maybe a lesser yield (in terms of
number of bytes) than expected?
I'll get it to you in a minute.
…On Mon, May 4, 2020, 00:48 Ian Lance Taylor ***@***.***> wrote:
Thanks. I see that the read calls are being interrupted, but I don't see
the calls that created those file descriptors. They are likely to be just
shortly before the traces you show. What I'm wondering is whether those
files are on an unusual file system, such as /sys.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#38836 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFOW2QP2WDF7757XGZMBRDRPZCJRANCNFSM4MYGYLUQ>
.
|
This corresponds to trace (2) above (about twenty lines before):
As a kludge, I added a loop in the scripting that's calling this tool to loop whenever it sees this failure. It looks like, that, when it occurs, it may perfectly reproduce, at least under similar conditions. It was likely exposed by network latency. Your guess/suggestion was very good.
|
Thanks for the info. It's the use of CIFS that causes this. I think the only fix is to fix #20400. |
That sucks. This is happening in the context of |
Yes, it can happen in general. For 1.14 you can make the problem less likely to occur by setting |
What's the source of the handler which didn't set SA_RESTART in this case? |
The handler did set |
Meaning a bug in Linux? Maybe a later kernel doesn't do this? Possibly related: |
Change https://golang.org/cl/232862 mentions this issue: |
I posted a 1.14 backport request for this: #39026 |
Fixes: #483 Related: golang/go#38836
Historically we've assumed that we can install all signal handlers with the SA_RESTART flag set, and let the system restart slow functions if a signal is received. Therefore, we don't have to worry about EINTR. This is only partially true, and we've added EINTR checks already for connect, and open/read on Darwin, and sendfile on Solaris. Other cases have turned up in golang#36644, golang#38033, and golang#38836. Also, golang#20400 points out that when Go code is included in a C program, the C program may install its own signal handlers without SA_RESTART. In that case, Go code will see EINTR no matter what it does. So, go ahead and check for EINTR. We don't check in the syscall package; people using syscalls directly may want to check for EINTR themselves. But we do check for EINTR in the higher level APIs in os and net, and retry the system call if we see it. This change looks safe, but of course we may be missing some cases where we need to check for EINTR. As such cases turn up, we can add tests to runtime/testdata/testprogcgo/eintr.go, and fix the code. If there are any such cases, their handling after this change will be no worse than it is today. For golang#22838 Fixes golang#20400 Fixes golang#36644 Fixes golang#38033 Fixes golang#38836 Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896 Reviewed-on: https://go-review.googlesource.com/c/go/+/232862 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> (cherry picked from commit 8c1db77)
I am running into EINTR failures with a CIFS mount as well, on go1.14. I cherry-picked 8c1db77 onto Before the cherry-pick, the errors were always in the read or write syscalls:
After the cherry-pick, the situation has improved but I still see occasional errors from
|
This fix isn't supported in 1.14, sadly. Can you try the 1.15 beta, and if it fails, file a new issue? |
Unfortunately I'm not able to do that since I don't control the machine/network where it occurs. If there are other related CLs I would appreciate a pointer to them. But from what I saw only the |
You probably also want https://golang.org/cl/236997. See #39237. |
What version of Go are you using (
go version
)?1.14.2
Does this issue reproduce with the latest release?
(This is the latest release)
What operating system and processor architecture are you using (
go env
)?linux, amd64
What did you do?
I'm running a Go tool of mine over a bunch of images. I'm getting periodic "interrupted system call" errors:
I've restarted several times. On earlier runs, I've gotten large clumps of these in the output (seems racy).
They always happen in the same place:
Is it possible that
ioutil.ReadFile()
is not correctly managing signals?(https://golang.org/pkg/os/signal/#hdr-Go_programs_that_use_cgo_or_SWIG)
The text was updated successfully, but these errors were encountered: