Commit Graph

809 Commits

Author SHA1 Message Date
Ian Lance Taylor c83d4fba07 os/signal: pass *int32 to ioctl that expects pid_t
Fixes #56233

Change-Id: I1cf176bc2f39c5e41d5a390ec6893426cdd39be0
Reviewed-on: https://go-review.googlesource.com/c/go/+/443175
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2022-10-17 15:26:00 +00:00
Michael Pratt 9ddb8ea737 os/signal: add missing newlines to TestTerminalSignal
For #37329.
For #56233.

Change-Id: Iafcddaddafd2d27fa5d535b57aaefec387f0b3f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/443066
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-14 20:48:50 +00:00
Michael Pratt a8ca70ff98 os/signal: rewrite TestTerminalSignal without bash
The existing version of this test contains several races it tries to
control with sleeps. Unfortunately, it is still flaky on darwin because
writing `fg` in bash too early can apparently result in failure to
actually continue the stopped child.

Rather than continuing to get perfect timing with bash, rewrite this to
eliminate bash and instead perform the same PTY operations that bash
would do.

This test is still quite complex because psuedo-terminals are
interminably complicated, but I believe it is no longer racy.
Technically there are still two races (waiting for child to enter read()
and waiting for the darwin kernel to wake the read after TIOCSPGRP), but
loss of either of these races should only mean we fail to test the
desired darwin EINTR case, not failure.

This test is skipped on DragonflyBSD, as it tickles a Wait hang bug
(#56132).

Updates #56132.
Fixes #37329.

Change-Id: I0ceaf5aa89f6be0f1bf68b2140f47db673cedb33
Reviewed-on: https://go-review.googlesource.com/c/go/+/440220
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
2022-10-14 15:34:49 +00:00
Bryan C. Mills 7feb68728d os: split wait6 syscall wrapper into per-platform files
There are getting to be enough special cases in this wrapper that
the increase in clarity from having a single file is starting to be
outweighed by the complexity from chained conditionals.

Updates #50138.
Updates #13987.

Change-Id: If4f1be19c0344e249aa6092507c28363ca6c8438
Reviewed-on: https://go-review.googlesource.com/c/go/+/442575
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-13 21:25:45 +00:00
Bryan C. Mills 498ee73a4b os/exec: reduce arbitrary sleeps in TestWaitid
If we use the "pipetest" helper command instead of "sleep",
we can use its stdout pipe to determine when the process
is ready to handle a SIGSTOP, and we can additionally check
that sending a SIGCONT actually causes the process to continue.

This also allows us to remove the "sleep" helper command,
making the test file somewhat more concise.

Noticed while looking into #50138.

Change-Id: If4fdee4b1ddf28c6ed07ec3268c81b73c2600238
Reviewed-on: https://go-review.googlesource.com/c/go/+/442576
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-13 20:21:32 +00:00
Bryan C. Mills 379a49c593 os/exec: set traceback to "system" in TestContextCancel
This will dump more goroutines if the test happens to fail.

For #50138.

Change-Id: Ifae30b5ba8bddcdaa9250dd90be8d8ba7d5604d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/442476
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-13 20:12:51 +00:00
Ian Lance Taylor 1a7f08cf40 os/signal: document behavior of SIGPIPE on non-Go thread
Fixes #56150

Change-Id: Id990783562950ba8be7ce9526b7a811625f2190a
Reviewed-on: https://go-review.googlesource.com/c/go/+/442415
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
2022-10-12 22:55:31 +00:00
Bryan C. Mills 9ec69908aa os: use the correct constant for P_PID on NetBSD
Dragonfly and FreeBSD both used numerical values for these constants
chosen to be the same as on Solaris. For some reason, NetBSD did not,
and happens to interpret value 0 as P_ALL instead of P_PID
(see 3323ceb782/sys/sys/idtype.h (L43-L44)).

Using the correct value for P_PID should cause wait6 to wait for the
correct process, which may help to avoid the deadlocks reported in

For #50138.
Updates #13987.

Change-Id: I0eacd1faee4a430d431fe48f9ccf837f49c42f39
Reviewed-on: https://go-review.googlesource.com/c/go/+/442478
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
2022-10-12 19:58:47 +00:00
Bryan C. Mills 0f64a49460 os/exec: remove protection against a duplicate Close on StdinPipe
As of CL 438347, multiple concurrents calls to Close should be safe.

This removes some indirection and may also make some programs that use
type-assertions marginally more efficient. For example, if a program
calls (*exec.Cmd).StdinPipe to obtain a pipe and then sets that as the
Stdout of another command, that program will now allow the second
command to inherit the file descriptor directly instead of copying
everything through a goroutine.

This will also cause calls to Close after the first to return an error
wrapping os.ErrClosed instead of nil. However, it seems unlikely that
programs will depend on that error behavior: if a program is calling
Write in a loop followed by Close, then if a racing Close occurs it is
likely that the Write would have already reported an error. (The only
programs likely to notice a change are those that call Close — without
Write! — after a call to Wait.)

Updates #56043.
Updates #9307.
Updates #6270.

Change-Id: Iec734b23acefcc7e7ad0c8bc720085bc45988efb
Reviewed-on: https://go-review.googlesource.com/c/go/+/439195
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-11 14:31:32 +00:00
Ian Lance Taylor 4fe1971b2d os: use poll.fdMutex for Plan 9 files
This permits us to safely support concurrent access to files on Plan 9.
Concurrent access was already safe on other systems.

This does introduce a change: if one goroutine calls a blocking read
on a pipe, and another goroutine closes the pipe, then before this CL
the close would occur. Now the close will be delayed until the blocking
read completes.

Also add tests that concurrent I/O and Close on a pipe are OK.

For #50436
For #56043

Change-Id: I969c869ea3b8c5c2f2ef319e441a56a3c64e7bf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/438347
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
2022-10-08 03:57:40 +00:00
hopehook fdea8e2170 os/exec: document ProcessState available after a call to Wait or Run
Wait or Run will populate its ProcessState when the command completes.

Fixes #56002.

Change-Id: I21547431f5d2d3e0fc0734fd1705421a0ac4209c
Reviewed-on: https://go-review.googlesource.com/c/go/+/437996
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-07 02:16:49 +00:00
Bryan C. Mills 515e3de299 os/exec: parallelize more tests
This cuts the wall duration for 'go test os/exec' and
'go test -race os/exec' roughly in half on my machine,
which is an even more significant speedup with a high '-count'.

For better or for worse, it may also increase the repro rate
of #34988.

Tests that use Setenv or Chdir or check for FDs opened during the test
still cannot be parallelized, but they are only a few of those.

Change-Id: I8d284d8bff05787853f825ef144aeb7a4126847f
Reviewed-on: https://go-review.googlesource.com/c/go/+/439196
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-06 19:09:18 +00:00
Bryan C. Mills 274d3a06f7 os/exec: delete TestExtraFilesFDShuffle
This test has been disabled for over nine years (since CL 12869049).
Although it still compiles, it seems likely to have rotted since then,
and if it was going to detect a real bug it also seems like that bug
would have been encountered and reported by users since then (and
would presumably have its own regression tests).

To eliminate overhead from mainining it (or skipping over it while
maintaining other tests), let's just delete it.

Fixes #5780.

Change-Id: I2a85cba20cba98a1dc6fc82336ae5e22d2242e99
Reviewed-on: https://go-review.googlesource.com/c/go/+/439197
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-06 18:18:50 +00:00
Ian Lance Taylor 755a2927d8 os: if dirFS.Open fails, undo use of backslashes in error message
This fixes a bug introduced by CL 426094 that caused the
golang.org/x/website/internal/web tests to fail.

Fixes #56034

Change-Id: Ic64967c6d440ad260b7283a18972b20023320ab6
Reviewed-on: https://go-review.googlesource.com/c/go/+/437976
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-06 02:55:59 +00:00
kijimaD c433cf1893 all: fix some typos
Change-Id: I6be77e7b7c919f26bed7b6690cce6741888ba78a
GitHub-Last-Rev: 4ef4a7b425
GitHub-Pull-Request: golang/go#56051
Reviewed-on: https://go-review.googlesource.com/c/go/+/438991
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-06 02:49:55 +00:00
Bryan C. Mills 0fec65d281 os/exec: add a GODEBUG setting to diagnose leaked processes
Updates #52580.
For #50436.

Change-Id: I669f13863f1f85d576c3c94500b118e6989000eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/436655
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
2022-10-04 23:19:13 +00:00
Bryan Mills 3380ee2520 Revert "os/exec: make StdoutPipe and StderrPipe safe to Close concurrently"
This reverts CL 437176.

Reason for revert: broke programs that plumb StdoutPipe from one command to Stdin on another and then call Wait on the former.

os/exec itself uses a type-assertion to *os.File to determine whether to copy stdin using a goroutine or just pass a file descriptor. An early Wait using a *os.File is benign (because closing the pipe doesn't close the child's inherited file descriptor), but an early Wait using a non-*os.File is not.

Updates #50436.

Change-Id: I4a2993e290982834f91696d890dfe77364c0cc50
Reviewed-on: https://go-review.googlesource.com/c/go/+/438695
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-10-04 21:17:23 +00:00
Ian Lance Taylor 82e357d6d5 os: use backslashes for DirFS on Windows
Otherwise DirFS of a UNC path does not work.

Fixes #54694

Change-Id: I82c1c436f7c26b3935c2cc4fd238daf094fc4d86
Reviewed-on: https://go-review.googlesource.com/c/go/+/426094
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: hopehook <hopehook@golangcn.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-10-02 02:31:18 +00:00
Bryan C. Mills 9a029a6915 os/exec: make StdoutPipe and StderrPipe safe to Close concurrently
For #50436, I want to be able to close the pipes returned by
StdoutPipe and StderrPipe after the Context has been canceled
and the WaitDelay has subsequently expired.

However, the fact that the exec.onceCloser wrapper for StdinPipe
(added in CL 13329043) was retained in CL 65490 suggests to me that
(*os.File).Close is still not safe to call concurrently.

This may cause type assertions of these ReadClosers to *os.File that
once succeeded to no longer do so. However, the StdoutPipe and
StderrPipe methods return interfaces, not concrete *os.Files, so
callers already should not have been relying on that implementation
detail — and as far as I can tell the closeOnce wrapper does not mask
any (*os.File) methods, so assertions to any interface type that
previously succeeded will continue to do so.

This change is logically part of CL 401835, but since it may expose
fragile type-assertions in callers I want to keep it separate for
clearer bisection of any new test failures.

For #50436.

Change-Id: I58de1d48fb6fd788502f13657d8d4484516271cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/437176
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-01 00:21:25 +00:00
Bryan C. Mills c9a62b7e71 os/exec: recombine goroutinePipes and userPipes
This change undoes CL 401894, because on further consideration
it turns out not to be needed.

This also makes (*Cmd).closeDescriptors a free function, since it does
not actually use the receiver in any way and is not needed to satisfy
any interfaces.

For #50436.

Change-Id: I7915265b0e6398ed5a34fae0c12873ab08a14194
Reviewed-on: https://go-review.googlesource.com/c/go/+/437175
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-01 00:12:32 +00:00
Tobias Klauser cc90e45f1d all: use "unix" build tag where appropriate
Convert a few occurrences that were submitted after CL 389935.

For #20322
For #51572

Change-Id: I0047361916c402f8e37f515e6b09d451bd499e6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/437235
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-09-30 23:58:34 +00:00
hopehook 972496ae6e all: use strings.Builder where appropriate
Change-Id: I164d350ca480640996055dedf38d962921c474a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/435975
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: hopehook <hopehook@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-29 22:56:00 +00:00
Bryan C. Mills ce3a5c0d10 os/exec: avoid leaking a process in TestDoubleStartLeavesPipesOpen
Updates #52580.
For #50436.

Change-Id: I0929055ffca1ca429f6ebec7d877f4268bd1fda2
Reviewed-on: https://go-review.googlesource.com/c/go/+/436656
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
2022-09-29 18:49:15 +00:00
Bryan C. Mills 223a563f58 os/exec: refactor goroutine error reporting
Use a separate channel to report the final error of the copying
goroutines, receiving a value only when all of the copying goroutines
have completed. In a followup change (CL 401835), that will allow
waiters to select on goroutine completion alongside other events (such
as Context cancellation).

Also mildly refactor the watchCtx helper method so that its structure
better matches what will be needed to implement WaitDelay.

For #50436.

Change-Id: I54b3997fb6931d204814d8382f0a388a67b520f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/435995
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
2022-09-29 02:34:30 +00:00
Bryan C. Mills 4d6ca68a85 os/exec: do not close pipes on a double-Start error
This fixes a bug introduced in CL 401834 in which calling Start twice
with pipes attached to a command would spuriously close those pipes
when returning the error from the second Start call.

For #50436.

Change-Id: I3563cc99c0a0987752190fef95da3e9927a76fda
Reviewed-on: https://go-review.googlesource.com/c/go/+/436095
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
2022-09-29 02:28:26 +00:00
Zeke Lu d7e663d909 os/signal: disable GNU readline to deflake TestTerminalSignal
Fixes #55903.

Change-Id: I992865277fb6526929d6c7db2b3b8d22ca0760f2
GitHub-Last-Rev: fc6f28e17c
GitHub-Pull-Request: golang/go#55904
Reviewed-on: https://go-review.googlesource.com/c/go/+/435735
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
2022-09-28 16:30:21 +00:00
cui fliter 190973eb60 all: fix some typos
Change-Id: I8b28aebbb9494b2c877139a4584a5a42253e3bea
GitHub-Last-Rev: e3703fd3a5
GitHub-Pull-Request: golang/go#55902
Reviewed-on: https://go-review.googlesource.com/c/go/+/435617
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-09-28 15:13:20 +00:00
Than McIntosh 07bdf1dc54 runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like

  func addExitHook(f func(), runOnNonZeroExit bool)

The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.

Updates #51430.

Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/354790
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2022-09-26 21:49:14 +00:00
Bryan C. Mills bd8a5b00fc os/exec: split parent I/O pipes by whether they are pumped by user code or internal goroutines
The pipes pumped by goroutines can be closed as soon as their
respective goroutines are done.

The pipes pumped by user code, however, are documented to be closed in
Wait. When we add the WaitDelay field, it isn't obvious that we should
terminate the user-pumped pipes when the WaitDelay expires, since Wait
itself isn't going to wait for those user-controlled goroutines to
complete.

(It's a bit more complicated than that because the documentation
currently states that Wait must not be called while the pipes are
being read — but it isn't obvious to me that that advice is entirely
correct.)

For #50436.

Change-Id: I97909c91d2097fb75138a360747168c08609696d
Reviewed-on: https://go-review.googlesource.com/c/go/+/401894
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
2022-09-26 17:26:59 +00:00
Bryan C. Mills b8d4a14a66 os/exec: clean up pipe-closing logic
Change the childFiles field to a local variable, since it was
populated during Start and (as far as I can determine) has no purpose
after Start returns.

Rename closeAfterStart and closeAfterWait to childIOFiles and
parentIOPipes respectively. That makes their contents clearer, and also
helps to clarify what should happen on error (when, for example, Wait
shouldn't be called at all).

Use a deferred call instead of individual calls to close child (and,
if necessary, pipe) FDs after Start. That helps to clarify the
invariants around when they are closed, and also makes the function a
bit more robust for future refactoring.

Also nil out the slices containing the file closers so that they can
be collected earlier.

This CL is intended as a pure refactor in preparation for #50436.

Change-Id: I05d13fa91d539b95b84b2ba923c1733f9a6203e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/401834
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
2022-09-26 17:26:56 +00:00
Tobias Klauser d74bf73be0 os: use wait6 to avoid wait/kill race on netbsd
Resend of CL 315281 which was partially reverted by CL 354249 after the
original CL was suspected to cause test failures as reported in #48789.
It seems that both wait4 and wait6 lead to that particular deadlock, so
let's use wait6. That way we at least don't hit #13987 on netbsd.

Updates #13987
For #48789
For #50138

Change-Id: Iadc4a771217b7e9e821502e89afa07036e0dcb6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/431855
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-19 18:44:37 +00:00
cuiweixie dedce99c06 os: convert Process.isdone to atomic type
Change-Id: Ia3213d22678be0d56bf4f34dfe458441f7f5da97
Reviewed-on: https://go-review.googlesource.com/c/go/+/426077
Run-TryBot: Michael Pratt <mpratt@google.com>
Run-TryBot: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Jenny Rakoczy <jenny@golang.org>
2022-09-15 21:11:27 +00:00
Andy Pan 86477e507f os: add a test case of copying a file itself via io.Copy
Change-Id: Ib9746cb4f27625cb22620271b280d2da242b2fba
Reviewed-on: https://go-review.googlesource.com/c/go/+/428437
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-09-09 15:34:46 +00:00
Kir Kolyshkin 3585e9be67 os: fix wrong error msg from TestDoubleCloseError
When the type assertion fails, the test mistakenly prints the expected
(rather than the actual) type.

When the error string doesn't match, the text mistakenly prints the
original (rather than the converted) error (although there might not be
any difference in the output, the code looks wrong).

Fix both issues.

Change-Id: Ia7dd0632fc677f458fec25d899c46268a12f76e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/428916
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2022-09-08 21:19:00 +00:00
Tobias Klauser d75e91740a os: use unsafe.{Slice,StringData} instead of unsafeheader package
Change-Id: I213b078effa4b7049c44498d651de5b938e5404b
Reviewed-on: https://go-review.googlesource.com/c/go/+/428779
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: hopehook <hopehook@golangcn.org>
Run-TryBot: hopehook <hopehook@golangcn.org>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
2022-09-08 21:18:58 +00:00
Kir Kolyshkin e828fbdffe os/exec: skip TestFindExecutableVsNoexec if mount failed
Apparently, some testing environments do not allow root to mount tmpfs
(due to e.g. AppArmor profile disallowing mount(2) syscall).

Always skip the test if the mount has failed.

Fixes the test issue introduced by CL 414824.

Change-Id: Ic565d2e6f277f2926d85a351be7df2498ffba656
Reviewed-on: https://go-review.googlesource.com/c/go/+/429175
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2022-09-07 18:41:21 +00:00
Kir Kolyshkin 88149ed43e os: fix wrong error msg from TestReadClosed
If test would fail, the error message will have wrong error and its
type, because e is used after the failed type assertion.

To fix, use the original err.

While at it,
 - combine the checks for error type and value into one statement;
 - use the standard "got ..., want ..." format.

Fixes: 212d2f82e0 ("os: add ErrClosed, return for use of closed File")
Change-Id: I862a96607b461ab89cce6bed2443b28aa2c16468
Reviewed-on: https://go-review.googlesource.com/c/go/+/428915
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-09-07 13:59:41 +00:00
cuiweixie 6375f508a8 os: use strings.Builder
Change-Id: I610509aa35c345ff7fbb1fc94bf177ffbe934731
Reviewed-on: https://go-review.googlesource.com/c/go/+/428274
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
2022-09-07 06:18:48 +00:00
Kir Kolyshkin 2b8f214094 os/exec: LookPath: use eaccess for exec check on linux
Having an executable bit set for a binary is not enough for it to be
executable -- there might be more checks in the kernel. For example,
binaries on a filesystem mounted with "noexec" flag couldn't be
executed. There might be other scenarios involving ACLs, SELinux,
file capabilities, and so on.

As a result, LookPath might either find a non-executable (while going
over $PATH elements), or return a false positive that the argument
provided is an executable.

One possible fix would be to perform the check by using access(2)
syscall with X_OK flag.

Now, since access(2) uses real (rather than effective) uid and gid,
when used by a setuid or setgid binary, it checks permissions of the
(real) user who started the binary, rather than the actual effective
permissions. Therefore, using access with X_OK won't work as expected
for setuid/setgid binaries.

To fix this, modern platforms added ways to check against effective uid
and gid, with the most common being the faccessat(2) call with the
AT_EACCESS flag, as described by POSIX.1-2008 (in Linux, only
faccessat2(2) supports flags such as AT_EACCESS). Let's use it, and fall
back to checking permission bits if faccessat is not available.

Wrap the logic into unix.Eaccess, which is currently only implemented on
Linux. While many other OSes (Free/Net/OpenBSD, AIX, Solaris/Illumos, and
Darwin) do implement faccessat(2) with AT_EACCESS, it is not wired in
syscall package (except for AIX), so these platforms are left out for now.
In the future, eaccess can be implemented for these OSes, too.

Alas, a call to unix.Eaccess is not enough since we have to filter out
directories, so use both stat and Eaccess.

One minor change introduced by this commit is that LookPath and Command
now returns "is a directory" error when the argument contains a slash
and is a directory.  This is similar to what e.g. bash does on Linux:

	$ bash -c /etc
	bash: line 1: /etc: Is a directory

Add a test case, which, unfortunately, requires root, is specific to
Linux, and needs a relatively new kernel (supporting faccessat2).  Other
platforms either have different semantics for tmpfs with noexec, or have
different ways to set up a binary which has x bit set but nevertheless
could not be executed.

Change-Id: If49b6ef6bf4dd23b2c32bebec8832d83e511a4bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/414824
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
2022-09-07 01:09:45 +00:00
Andy Pan 979956a732 os: delete unused errClosed
errClosed was introduced by CL 163058 and was supposed to be removed by CL 243906,
but somehow it was left out, now we should get it deleted.

Change-Id: I74c4b36b8bbc4844e1860acb022a16b0aa3272b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/427035
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-09-06 16:57:58 +00:00
Yuval Pavel Zholkover b15c399a36 os: only add file descriptors which are set to non-blocking mode to the netpoller
Either ones where kind == kindNonBlock or those we've successfully called syscall.SetNonblock() on.
Restore blocking behavior if we detect an error registering with the netpoller and our flow was
successful in setting the inital syscall.SetNonblock().

Update #54100

Change-Id: I08934e4107c7fb36c15a7ca23ac880490b4df235
Reviewed-on: https://go-review.googlesource.com/c/go/+/420334
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Goutnik <dgoutnik@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Yuval Pavel Zholkover <paulzhol@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
2022-08-18 03:12:27 +00:00
Russ Cox 027855e8d8 os/exec: add GODEBUG setting to opt out of ErrDot changes
The changes are likely to break users, and we need
to make it easy to unbreak without code changes.

For #43724.
Fixes #53962.

Change-Id: I105c5d6c801d354467e0cefd268189c18846858e
Reviewed-on: https://go-review.googlesource.com/c/go/+/419794
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
2022-07-28 19:12:40 +00:00
Ian Lance Taylor 53a4152d47 os/exec: clarify that Wait must be called
Fixes #52580

Change-Id: Ib2dd8a793b9c6fcb083abb3f7c346f6279adefc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/414056
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-07-06 16:51:00 +00:00
AndreasHGK af725f4286 os: fix a typo in path_windows.go
I believe the path_windows.go file has a typo, which is fixed in this PR

Change-Id: Ibf1a7189a6312dbb3b1e6b512beeb6d99da5b5bc
GitHub-Last-Rev: cedac7eaa0
GitHub-Pull-Request: golang/go#53629
Reviewed-on: https://go-review.googlesource.com/c/go/+/415434
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
2022-06-30 21:52:06 +00:00
Ian Lance Taylor 993c387032 os: simplify deadline fluctuation tests
This applies the net package CL 365334, CL 366176, CL 372215 to the os
package.

CL 365334:

    These tests were checking for fairly narrow timing windows, but were
    running in parallel and heavily dependent on timer and goroutine
    scheduling. This change eliminates unnecessary goroutines, runs the
    tests sequentially (dramatically shortening the timeouts to reduce the
    penalty of doing so), and uses timestamp comparison instead of
    background timers to hopefully gain some robustness from monotonic
    timestamps.

    Many of the other tests from this package would benefit from similar
    simplifications, which we can apply if and when we notice flaky
    failures or want to improve the latency of running the test.

CL 366176:

    It appears that at least the OpenBSD kernel gets sloppier the longer
    the timeout we give it, up to an observed overhead of around 25%.
    Let's give it a little more than that (33%) in the comparison, and
    also increase the growth curve to match the actual observed times
    instead of exponential initial growth.

CL 372215:

    Decrease the slop everywhere else, since NetBSD and OpenBSD seem to be
    the only ones that miss by that much.

For #36108
For #50189
Fixes #50725 (we hope)

Change-Id: I0854d27af67ca9fcf0f9d9e4ff67acff4c2effc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/415234
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-06-30 19:37:21 +00:00
Bryan C. Mills 3580ef9d64 os/exec: on Windows, suppress ErrDot if the implicit path matches the explicit one
If the current directory is also listed explicitly in %PATH%,
this changes the behavior of LookPath to prefer the explicit name for it
(and thereby avoid ErrDot).

However, in order to avoid running a different executable from what
would have been run by previous Go versions, we still return the
implicit path (and ErrDot) if it refers to a different file entirely.

Fixes #53536.
Updates #43724.

Change-Id: I7ab01074e21a0e8b07a176e3bc6d3b8cf0c873cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/414054
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-06-28 19:29:51 +00:00
Dan Kortschak b72a6a7b86 os: document that Chdir affects fs.FS returned by DirFS with a relative path
Fixes #47214.

Change-Id: I6fdc1c4340c0943b825ac22e311179ad1cf30915
Reviewed-on: https://go-review.googlesource.com/c/go/+/410334
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-06-07 23:09:21 +00:00
Xiaodong Liu 1371339530 go, math, os, reflect: support standard library for loong64
Contributors to the loong64 port are:
  Weining Lu <luweining@loongson.cn>
  Lei Wang <wanglei@loongson.cn>
  Lingqin Gong <gonglingqin@loongson.cn>
  Xiaolin Zhao <zhaoxiaolin@loongson.cn>
  Meidan Li <limeidan@loongson.cn>
  Xiaojuan Zhai <zhaixiaojuan@loongson.cn>
  Qiyuan Pu <puqiyuan@loongson.cn>
  Guoqi Chen <chenguoqi@loongson.cn>

This port has been updated to Go 1.15.6:
  https://github.com/loongson/go

Updates #46229

Change-Id: I2ad9ed01fc913b90e75023ac0fa70de87a9f5de1
Reviewed-on: https://go-review.googlesource.com/c/go/+/342324
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2022-05-20 15:12:52 +00:00
Silke Hofstra 12e48f3bbf os: look up hostname from PATH in test
When running TestHostname, the location of the hostname binary
is hardcoded as /bin/hostname. However, on some systems the actual
location is /usr/bin/hostname.

Change this behaviour to perform a lookup for hostname in PATH,
and skip the test when it cannot be found there.

Fixes #52402

Change-Id: I5418bf77258f5ffb2a9f834b8c68d8a7b7a452d7
GitHub-Last-Rev: 750f36fcf9
GitHub-Pull-Request: golang/go#52403
Reviewed-on: https://go-review.googlesource.com/c/go/+/400794
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-05-18 15:19:51 +00:00
John Bampton 20db15ce12 all: fix spelling
Change-Id: I63eb42f3ce5ca452279120a5b33518f4ce16be45
GitHub-Last-Rev: a88f2f72be
GitHub-Pull-Request: golang/go#52951
Reviewed-on: https://go-review.googlesource.com/c/go/+/406843
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
2022-05-18 00:47:29 +00:00