internal/poll: deadlock in Read on arm64 when an FD is closed [1.17 backport]

[gopherbot]

@bcmills requested issue #45211 to be considered for backport to the next 1.17 minor release.

@gopherbot, please backport to 1.17 and 1.16: this is a race condition that can result in deadlocks on ARM64 in any process that relies on Close unblocking a read. It is tricky to diagnose (and hard to reproduce reliably), and there is no apparent workaround for users on ARM64 machines.

[rsc]

I'd feel better about a backport if we had more data showing that the crashes really have stopped.

[bcmills]

Unfortunately the crashes only reproduced on a very small number of builders, and even then not in a way we could reproduce easily outside of the builder environment, which can be arbitrarily different from even a gomote due to system load, test sharding, and the process environment (see #32430). To make matters worse, because of the reduced rate of CLs for the code freeze we aren't getting very many organic test runs on the builders we do have.

Perhaps we should revisit a backport after the tree reopens for 1.19 development, so that we can have a larger body of test runs to compare?

[toothrot]

Approved. This is a serious issue with no workaround.

[dmitshur]

https://go.dev/wiki/MinorRelease states:

When the child issue is labeled CherryPickApproved, the original author of the change fixing that issue should immediately create and mail a cherry-pick change against the release branch, which will be merged as soon as it is ready, closing the child issue.

Both this issue and #50610 are now labeled CherryPickApproved.

@rsc You're the original author of CL 378234 that fixed #45211, so please feel free to mail cherry-pick CLs when you're ready to proceed here. Thanks.

[heschi]

@bcmills @rsc @ianlancetaylor

This backport is at risk of missing the next minor release.

[gopherbot]

Change https://go.dev/cl/392714 mentions this issue: [release-branch.go1.17] runtime: fix net poll races

[rhysh]

Unfortunately the crashes only reproduced on a very small number of builders, and even then not in a way we could reproduce easily outside of the builder environment

I'm not sure whether disabling arm64HasATOMICS = cpu.ARM64.HasATOMICS in src/runtime/proc.go is "allowed" (breaks invariants), but doing that has let me reproduce a related issue (#50469) on my darwin/arm64 M1 Macbook Air about once per minute. It reproduces up until the fix for #45211 , at 17b2fb1.

So if falling back to older atomic instructions is a valid way to run, then it looks to me like that commit really does stop the crashes (at least by a factor of 50, from ~3% failure rate to 0 out of 2000).

Update: I ran the reproducer 45k times, with no netpoll-related deadlocks. (The single failure I saw was reported as a normal application-layer TCP timeout.)

[gopherbot]

Closed by merging 4e69fdd to release-branch.go1.17.