Skip to content

os/exec: TestCatGoodAndBadFile failures on illumos #68863

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
Closed
panjf2000 opened this issue Aug 14, 2024 · 6 comments
Closed

os/exec: TestCatGoodAndBadFile failures on illumos #68863

panjf2000 opened this issue Aug 14, 2024 · 6 comments
Assignees
@panjf2000
Labels
NeedsFix The path to resolution is known, but the work has not been done. OS-illumos Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.24

Comments

@panjf2000
Copy link
Member

panjf2000 commented Aug 14, 2024 •
edited
Loading 

--- FAIL: TestCatGoodAndBadFile (0.01s)
    exec_test.go:430: expected test code; got "" (len 0)
FAIL
FAIL	os/exec	0.510s
ok  	os/exec/internal/fdtest	0.012s
FAIL
go tool dist: Failed: exit status 1


Error: tests failed: dist test failed: {go_test:os os}: exit status 1

sendfile(3ext) on illumos seems to incur intermittent failures when the target file is the standard stream (stdout, stderr):

https://build.golang.org/log/bb20485d1d72074792215facb7b28ae661e6a6e0
https://build.golang.org/log/2c88f9652638bc186037196f95f84506c2e139c7
https://build.golang.org/log/4de2769138b79cd750b41ea950b148ffb118f6ef

Related CL: https://go.dev/cl/603098

/cc @ianlancetaylor @neild
@panjf2000 panjf2000 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. OS-illumos labels Aug 14, 2024
@panjf2000 panjf2000 added this to the Go1.24 milestone Aug 14, 2024
@panjf2000 panjf2000 self-assigned this Aug 14, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/605355 mentions this issue: os: don't use sendfile(3ext) on illumos when target is standard stream

@panjf2000 panjf2000 changed the title os: TestCatGoodAndBadFile failures on illumos os/exec: TestCatGoodAndBadFile failures on illumos Aug 14, 2024
@panjf2000 panjf2000 moved this to Active in Test Flakes Aug 14, 2024
@gopherbot
Copy link
Contributor

Sorry, but I can't find a watchflakes script at the start of the issue description.
See https://go.dev/wiki/Watchflakes for details.

— watchflakes

@github-project-automation github-project-automation bot moved this from Active to Done in Test Flakes Aug 14, 2024
@panjf2000
Copy link
Member Author

Looks like this flake didn't go away: https://build.golang.org/log/876e7c984d928194c55935da414eeb30e514473f

@panjf2000 panjf2000 reopened this Aug 16, 2024
@github-project-automation github-project-automation bot moved this from Done to Active in Test Flakes Aug 16, 2024
@panjf2000
Copy link
Member Author

/cc @ianlancetaylor

@panjf2000
Copy link
Member Author

panjf2000 commented Aug 16, 2024 •
edited
Loading

It seems that stdout in exec_test.go on illumos turns out not to be a character device somehow. Maybe there is something like fd hijacking of stdout somehow? Despite sendfile() works for pipes on illumos in practice, it's not documented in the man pages, it instead only explicitly claims "The out_fd argument should be a file descriptor to a regular file opened for writing or to a connected AF_INET or AF_INET6 socket of SOCK_STREAM type", which is endorsed by the source code of the illumos kernel. Therefore, maybe we should be more conservative and only allow syscall.S_IFREG to avoid any uncanny cases.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/606135 mentions this issue: os: only employ sendfile(3ext) on illumos when target is regular file

@github-project-automation github-project-automation bot moved this from Active to Done in Test Flakes Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@panjf2000 panjf2000

Labels
NeedsFix The path to resolution is known, but the work has not been done. OS-illumos Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Test Flakes
Archived in project
Milestone
Go1.24
Development

No branches or pull requests
2 participants
@panjf2000 @gopherbot