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/oauth2/authhandler: Example persistently deadlocking on Android builders due to reading from os.Stdin #45523

Closed
bcmills opened this issue Apr 12, 2021 · 2 comments
Closed

x/oauth2/authhandler: Example persistently deadlocking on Android builders due to reading from os.Stdin #45523

bcmills opened this issue Apr 12, 2021 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.17

Comments

@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2021 •
edited

The Example test added to x/oauth2/authhandler in CL 232238 uses fmt.Scanln to read from os.Stdin:
https://go.googlesource.com/oauth2/+/refs/heads/master/authhandler/example_test.go#35

The assumption that an Example function can safely read from os.Stdin — or that os.Stdin is either closed or unclosed during the execution of a test binary — does not hold in general.

The Example function should either be fixed or removed. Note that its current failure mode is to always time out, which can waste substantial builder resources (CC @golang/release).

2021-04-02T16:14:24-2e8d934/android-386-emu
2021-04-02T16:14:24-2e8d934/android-amd64-emu
2021-03-23T18:09:02-22b0ada/android-386-emu
2021-03-23T18:09:02-22b0ada/android-amd64-emu

CC @shinfan @andyrzhao @tbpg @codyoss
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. Soon This needs to be done soon. (regressions, serious bugs, outages) labels Apr 12, 2021
@gopherbot gopherbot added this to the Unreleased milestone Apr 12, 2021
@bcmills bcmills modified the milestones: Unreleased, Go1.17 Apr 12, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Apr 12, 2021

Marking as release-blocker for Go 1.17 via #11811.

@andyrzhao andyrzhao mentioned this issue Apr 12, 2021
Closed
gopherbot pushed a commit to golang/oauth2 that referenced this issue Apr 13, 2021
@andyrzhao
authhandler: Remove example_test.go
5e61552 
The example test implementation uses stdin, which does not work in all environments:
golang/go#45523

Removing for now to unblock release.

Change-Id: I49bafa9fe1d973b7c1d7ce00f51f110f9aa4a5a6
GitHub-Last-Rev: baf4632
GitHub-Pull-Request: #488
Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/309469
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Trust: Cody Oss <codyoss@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@bcmills
Copy link
Contributor Author

bcmills commented Apr 13, 2021

Fixed by removing the Example function in CL 309469.

@bcmills bcmills closed this as completed Apr 13, 2021
@golang golang locked and limited conversation to collaborators Apr 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
2 participants
@bcmills @gopherbot