This is an attempt to distinguish between a dropped signal and
general builder slowness.
The previous attempt (increasing the settle time to 250ms) still
resulted in a timeout:
https://build.golang.org/log/dd62939f6d3b512fe3e6147074a9c6db1144113f
For #33174
Change-Id: I79027e91ba651f9f889985975f38c7b01d82f634
Reviewed-on: https://go-review.googlesource.com/c/go/+/228266
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestExtraFiles seems to be flaky on GNU/Linux systems when using cgo
because creating a new thread will call malloc which can create a new
arena which can open a file to see how many processors there are.
Try to avoid the flake by creating several new threads at process
startup time.
For #25628
Change-Id: Ie781acdbba475d993c39782fe172cf7f29a05b24
Reviewed-on: https://go-review.googlesource.com/c/go/+/228099
Reviewed-by: Bryan C. Mills <bcmills@google.com>
I noticed a timeout in TestIgnore in
https://build.golang.org/log/52d83a72f3a5ea9a16eb5d670c729694144f9624,
which suggests that the settle time is currently set too low.
I've also added a check for the same GO_TEST_TIMEOUT_SCALE used in
TestTerminalSignal, so that if this builder remains too slow we can
increase the builder's scale factor rather than the test's baseline
running time.
Updates #33174
Change-Id: I18b10eaa3bb5ae2f604300aedaaf6f79ee7ad567
Reviewed-on: https://go-review.googlesource.com/c/go/+/227649
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Try to get some output even if the subprocess hangs.
For #25628
Change-Id: I4cc0a8f2c52b03a322b8fd0a620cba37b06ff10a
Reviewed-on: https://go-review.googlesource.com/c/go/+/227517
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This removes all conditions and conditional code (that I could find)
that depended on darwin/386.
Fixes#37610.
Change-Id: I630d9ea13613fb7c0bcdb981e8367facff250ba0
Reviewed-on: https://go-review.googlesource.com/c/go/+/227582
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes all conditions and conditional code (that I could find)
that depended on darwin/arm.
Fixes#35439 (since that only happened on darwin/arm)
Fixes#37611.
Change-Id: Ia4c32a5a4368ed75231075832b0b5bfb1ad11986
Reviewed-on: https://go-review.googlesource.com/c/go/+/227198
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We can initialize the runtime sigqueue packages on first use.
We don't require an explicit initialization step. So, remove it.
Change-Id: I484e02dc2c67395fd5584f35ecda2e28b37168df
Reviewed-on: https://go-review.googlesource.com/c/go/+/226540
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
For reasons unknown, SIGUSR1 appears to be blocked at process start
for tests on the android-arm-corellium and android-arm64-corellium
builders. (This has been observed before, too: see CL 203957.)
Make the test resilient to blocked signals by always calling Notify
and waiting for potential signal delivery after sending any signal
that is not known to be unblocked.
Also remove the initial SIGWINCH signal from testCancel. The behavior
of an unhandled SIGWINCH is already tested in TestStop, so we don't
need to re-test that same case: waiting for an unhandled signal takes
a comparatively long time (because we necessarily don't know when it
has been delivered), so this redundancy makes the overall test binary
needlessly slow, especially since it is called from both TestReset and
TestIgnore.
Since each signal is always unblocked while we have a notification
channel registered for it, we don't need to modify any other tests:
TestStop and testCancel are the only functions that send signals
without a registered channel.
Fixes#38165
Updates #33174
Updates #15661
Change-Id: I215880894e954b62166024085050d34323431b63
Reviewed-on: https://go-review.googlesource.com/c/go/+/226461
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In CL 226138, I updated TestStop to have more uniform behavior for its signals.
However, that test seems to always fail for SIGUSR1 on the Android ARM builders.
I'm not sure what's special about Android for this particular case,
but let's skip the test to unbreak the builders while I investigate.
For #38165
Updates #33174
Change-Id: I35a70346cd9757a92acd505a020bf95e6871405c
Reviewed-on: https://go-review.googlesource.com/c/go/+/226458
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use a uniform function (named “quiesce”) to wait for possible signals
in a way that gives the kernel many opportunities to deliver them.
Simplify channel usage and concurrency in stress tests.
Use (*testing.T).Deadline instead of parsing the deadline in TestMain.
In TestStop, sleep forever in a loop if we expect the test to die from
a signal. That should reduce the flakiness of TestNohup, since
TestStop will no longer spuriously pass when run as a subprocess of
TestNohup.
Since independent signals should not interfere, run the different
signals in TestStop in parallel when testing in short mode.
Since TestNohup runs TestStop as a subprocess, and TestStop needs to
wait many times for signals to quiesce, run its test subprocesses
concurrently and in short mode — reducing the latency of that test by
more than a factor of 2.
The above two changes reduce the running time of TestNohup on my
workstation to ~345ms, making it possible to run much larger counts of
the test in the same amount of wall time. If the test remains flaky
after this CL, we can spend all or part of that latency improvement on
a longer settle time.
Updates #33174
Change-Id: I09206f213d8c1888b50bf974f965221a5d482419
Reviewed-on: https://go-review.googlesource.com/c/go/+/226138
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On linux-386 builders run the TestExtraFiles subprocess under strace,
in hopes of finding out where the unexpected descriptor is coming from.
For #25628
Change-Id: I9a62d6a5192a076525a616ccc71de74bbe7ebd58
Reviewed-on: https://go-review.googlesource.com/c/go/+/225799
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
For #25628
Change-Id: If1dce7ba9310e1418e67b9954c989471b775a28e
Reviewed-on: https://go-review.googlesource.com/c/go/+/225278
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Several method implementations were identical in file_unix.go and
file_windows.go. Merge them into file_posix.go.
Change-Id: I8bcfad468829530f81f52fe426b3a8c042e7bbd6
Reviewed-on: https://go-review.googlesource.com/c/go/+/224138
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
When we seek on the underlying FD, discard any directory entries
we've already read and cached. This makes sure we won't return
the same entry twice.
We already fixed this for Darwin in CL 209961.
Fixes#37161
Change-Id: I20e1ac8d751443135e67fb4c43c18d69befb643b
Reviewed-on: https://go-review.googlesource.com/c/go/+/219143
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
NodeJS does not support fchdir so it has to be emulated with chdir by
saving the path when opening a directory.
However, if the path opened is relative, saving this path is not
sufficient, because after changing the working directory the path
does not resolve correctly any more, thus a subsequent fd.Chdir() fails.
This change fixes the issue by resolving a relative path when
opening the directory and saving the absolute path instead.
Fixes#37448
Change-Id: Id6bc8c4232b0019fc11e850599a526336608ce54
Reviewed-on: https://go-review.googlesource.com/c/go/+/221717
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.
Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Builds upon the changes from #32000 which supported sourcing environment
variables for a new process from the environment of a Windows user token
when supplied.
But due to the logic of os/exec, the Env field of a process was
always non-nil when it reached that change.
This change moves the logic up to os/exec, specifically when
os.ProcAttr is being built for the os.StartProcess call, this
ensures that if a user token has been supplied and no Env slice has
been provided on the command it will be sourced from the user's
environment.
If no token is provided, or the program is compiled for any other
platform than Windows, the default environment will be sourced from
syscall.Environ().
Fixes#35314
Change-Id: I4c1722e90b91945eb6980d5c5928183269b50487
GitHub-Last-Rev: 32216b7291
GitHub-Pull-Request: golang/go#37402
Reviewed-on: https://go-review.googlesource.com/c/go/+/220587
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts CL 214437.
Does not fix the issue, and the test was wrong so it did not detect that it did not fix the issue.
Updates #36375
Change-Id: I6a4112035a1e90f4fdafed6fdf4ec9dfc718b571
Reviewed-on: https://go-review.googlesource.com/c/go/+/214601
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#36375
Change-Id: I407a1db23868880b83e73bc136d274659483fb69
Reviewed-on: https://go-review.googlesource.com/c/go/+/214437
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This test was recently added in CL 209961.
Apparently Windows can't seek a directory filehandle?
And move the test from test/fixedbugs (which is mostly for compiler bugs) to
an os package test.
Updates #36019
Change-Id: I626b69b0294471014901d0ccfeefe5e2c7651788
Reviewed-on: https://go-review.googlesource.com/c/go/+/210283
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This change replaces
buf := [HUGE_CONST]*T)(unsafe.Pointer(p))[:]
with
buf := [HUGE_CONST]*T)(unsafe.Pointer(p))[:n:n]
Pointer p points to n of T elements. New unsafe pointer conversion
logic verifies that both first and last elements point into the same
Go variable.
This change replaces [:] with [:n:n] to please pointer checker.
According to @mdempsky, compiler specially recognizes when you
combine a pointer conversion with a full slice operation in a single
expression and makes an exception.
After this, only one failure in net remains when running:
go test -a -short -gcflags=all=-d=checkptr std cmd
Updates #34972
Change-Id: I2c8731650c856264bc788e4e07fa0530f7c250fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/208617
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The first Readdirnames calls opendir and caches the result.
The behavior of that cached opendir result isn't specified on a seek
of the underlying fd. Free the opendir result on a seek so that
we'll allocate a new one the next time around.
Also fix wasm behavior in this regard, so that a seek to the
file start resets the Readdirnames position, regardless of platform.
p.s. I hate the Readdirnames API.
Fixes#35767.
Change-Id: Ieffb61b3c5cdd42591f69ab13f932003966f2297
Reviewed-on: https://go-review.googlesource.com/c/go/+/209961
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#35085
Change-Id: Ice611e1223392f687061a43fd4c2298ea22774fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/207081
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Fixes#35492
Change-Id: I00dce8fd1228f809e0c61013ac4de7a5953cbbf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/206997
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It turns out that there is a path that initializes netpoll and opens
file descriptors before running the os/exec init function: on some
systems, the uses of NewFile when setting os.Stdin and friends can
initialize netpoll which can open file descriptors. This in itself
is not a problem, but when we check whether the new files are open
using os.NewFile, a side-effect is to put them into non-blocking mode.
This can then break future uses of netpoll.
Updates #35469Fixes#35566
Change-Id: I1b2e2c943695d1c2d29496b050abbce9ee710a00
Reviewed-on: https://go-review.googlesource.com/c/go/+/207078
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Our attempts to close existing open files are flaky. They will fail if,
for example, file descriptor 3 is open when the test binary starts.
Instead, report any such cases, and skip TestExtraFiles.
Updates #35469
Change-Id: I7caec083f3f4a31579bf28fc9c82ae89b1bde49a
Reviewed-on: https://go-review.googlesource.com/c/go/+/206939
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
os.OpenFile was assuming that a failed syscall.Open means the file does
not exist and it tries to create it. However, syscall.Open may have
failed for some other reason, such as failing to lock a os.ModeExclusive
file. We change os.OpenFile to only create the file if the error
indicates that the file doesn't exist.
Remove skip of TestTransform test, which was failing because sometimes
syscall.Open would fail due to the file being locked, but the
syscall.Create would succeed because the file is no longer locked. The
create was truncating the file.
Fixes#35471
Change-Id: I06583b5f8ac33dc90a51cc4fb64f2d8d9c0c2113
Reviewed-on: https://go-review.googlesource.com/c/go/+/206299
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Now that the runtime can send preemption signals, it is possible that
a channel that asks for all signals can see both SIGURG and SIGHUP
before reading either, in which case one of the signals will be dropped.
We have to use a larger buffer so that the test see the signal it expects.
Fixes#35466
Change-Id: I36271eae0661c421780c72292a5bcbd443ada987
Reviewed-on: https://go-review.googlesource.com/c/go/+/206257
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This adds support for pausing a running G by sending a signal to its
M.
The main complication is that we want to target a G, but can only send
a signal to an M. Hence, the protocol we use is to simply mark the G
for preemption (which we already do) and send the M a "wake up and
look around" signal. The signal checks if it's running a G with a
preemption request and stops it if so in the same way that stack check
preemptions stop Gs. Since the preemption may fail (the G could be
moved or the signal could arrive at an unsafe point), we keep a count
of the number of received preemption signals. This lets stopG detect
if its request failed and should be retried without an explicit
channel back to suspendG.
For #10958, #24543.
Change-Id: I3e1538d5ea5200aeb434374abb5d5fdc56107e53
Reviewed-on: https://go-review.googlesource.com/c/go/+/201760
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Remove skipping of TestRemoveUnreadableDir on Windows.
Fixes#26295
Change-Id: I364a3caa55406c855ece807759f6298f7e4ddf1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/203599
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously we injected an error, and the injection points were
(empirically) not realistic on some platforms.
Instead, we now make the directory read-only, which (on most
platforms) suffices to prevent the removal of its files.
Fixes#35117
Updates #29921
Change-Id: Ica4e2818566f8c14df3eed7c3b8de5c0abeb6963
Reviewed-on: https://go-review.googlesource.com/c/go/+/203502
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, TestAtomicStop used a hard-coded 2-second timeout.
That empirically is not long enough on certain builders. Rather than
adjusting it to a different arbitrary value, use a slice of the
overall timeout for the test binary. If everything is working, we
won't block nearly that long anyway.
Updates #35085
Change-Id: I7b789388e3152413395088088fc497419976cf5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/203499
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On ARM and ARM64, during a VDSO call, the g register may be
temporarily clobbered by the VDSO code. If a signal is received
during the execution of VDSO code, we may not find a valid g
reading the g register. In CL 192937, we conservatively assume
g is nil. But this approach has a problem: we cannot handle
the signal in this case. Further, if the signal is not a
profiling signal, we'll call badsignal, which calls needm, which
wants to get an extra m, but we don't have one in a non-cgo
binary, which cuases the program to hang.
This is even more of a problem with async preemption, where we
will receive more signals than before. I ran into this problem
while working on async preemption support on ARM64.
In this CL, before making a VDSO call, we save the g on the
gsignal stack. When we receive a signal, we will be running on
the gsignal stack, so we can fetch the g from there and move on.
We probably want to do the same for PPC64. Currently we rely on
that the VDSO code doesn't actually clobber the g register, but
this is not guaranteed and we don't have control with.
Idea from discussion with Dan Cross and Austin.
Should fix#34391.
Change-Id: Idbefc5e4c2f4373192c2be797be0140ae08b26e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/202759
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
This was disabled due to a report that the App Store rejects the symbol
__sysctl. However, we use the sysctl symbol, which is fine. The __sysctl
symbol is used by x/sys/unix, which needs fixing instead. So, this
commit reenables sysctl on iOS, so that things like net.InterfaceByName
can work again.
This reverts CL 193843, CL 193844, CL 193845, and CL 193846.
Fixes#35101
Updates #34133
Updates #35103
Change-Id: Ib8eb9f87b81db24965b0de29d99eb52887c7c60a
Reviewed-on: https://go-review.googlesource.com/c/go/+/202778
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The issues associated with these skipped checks are closed.
If they are working around unfixed bugs, the issues should remain open.
If they are working around unfixable properties of the system, the skips
should refer to those properties rather than closed issues.
Updates #2603
Updates #3955
Updates #25628
Change-Id: I3491c69b2ef5bad0fb12001fe8f7e06b424883ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/201718
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also log errors from the lsof command on failure.
(That's how the missing environment was discovered.)
Updates #25628
Change-Id: I71594f60c15d0d254d5d4a86deec7431314c92ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/201717
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This timeout will never be reached if the test passes, so it doesn't
much matter how long it is. The test is t.Parallel so on a slow system
1 second may occasionally not be enough, although on my laptop the
test takes about 0.02 seconds.
Fixes#34431
Change-Id: Ia2184e6be3747933bfe83aa6c8e1f77e6b1e0bba
Reviewed-on: https://go-review.googlesource.com/c/go/+/200764
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
You were a useful port and you've served your purpose.
Thanks for all the play.
A subsequent CL will remove amd64p32 (including assembly files and
toolchain bits) and remaining bits. The amd64p32 removal will be
separated into its own CL in case we want to support the Linux x32 ABI
in the future and want our old amd64p32 support as a starting point.
Updates #30439
Change-Id: Ia3a0c7d49804adc87bf52a4dea7e3d3007f2b1cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/199499
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 197938 actually fixes those regression on Darwin as syscalls
are no longer labeled as always blocking and consume a thread.
Fixes#33953Fixes#32326
Change-Id: I82c98516c23cd36f762bc5433d7b71ea8939a0ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/199477
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
The existing text was hard to parse.
Shorten the sentences and simplify the text.
Change-Id: Ic16f486925090ea303c04e70969e5a4b27a60896
Reviewed-on: https://go-review.googlesource.com/c/go/+/198758
Reviewed-by: Ian Lance Taylor <iant@golang.org>
By lazily starting the signal watch loop only on Notify,
we are able to have deadlock detection even when
"os/signal" is imported.
Thanks to Ian Lance Taylor for the solution and discussion.
With this change in, fix a runtime gorountine count test that
assumed that os/signal.init would unconditionally start the
signal watching goroutine, but alas no more.
Fixes#21576.
Change-Id: I6eecf82a887f59f2ec8897f1bcd67ca311ca42ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/101036
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Try to deflake TestNohup.
The kernel will deliver a signal as a thread returns from a syscall.
If the only active thread is sleeping, and the system is busy,
the kernel may not get around to waking up a thread to catch the signal.
Try splitting up the sleep, to give the kernel another change to deliver.
I don't know if this will help, but it seems worth a try.
Fixes#33174
Change-Id: I34b3240af706501ab8538cb25c4846d1d30d7691
Reviewed-on: https://go-review.googlesource.com/c/go/+/194879
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Use the following (suboptimal) script to obtain a list of possible
typos:
#!/usr/bin/env sh
set -x
git ls-files |\
grep -e '\.\(c\|cc\|go\)$' |\
xargs -n 1\
awk\
'/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
hunspell -d en_US -l |\
grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
sort -f |\
uniq -c |\
awk '$1 == 1 { print $2; }'
Then, go through the results manually and fix the most obvious typos in
the non-vendored code.
Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
Some were never used, and some haven't been used for years.
One exception is net/http's readerAndCloser, which was only used in a
test. Move it to a test file.
While at it, remove a check in regexp that could never fire; the field
is an uint32, so it can never be negative.
Change-Id: Ia2200f6afa106bae4034045ea8233b452f38747b
Reviewed-on: https://go-review.googlesource.com/c/go/+/192621
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This matches the existing behavior of treating CTRL_C_EVENT, CTRL_BREAK_EVENT as a synthesized SIGINT event.
See https://docs.microsoft.com/en-us/windows/console/handlerroutine for a good documentation source upstream to confirm these values.
As for the usage of these events, the "Timeouts" section of that upstream documentation is important to note, especially the limited window in which to do any cleanup before the program will be forcibly killed (defaults typically 5s, but as low as 500ms, and in many cases configurable system-wide).
These events are especially relevant for Windows containers, where these events (particularly `CTRL_SHUTDOWN_EVENT`) are one of the only ways containers can "gracefully" shut down (https://github.com/moby/moby/issues/25982#issuecomment-466804071).
This was verified by making a simple `main()` which implements the same code as in `ExampleNotify_allSignals` but in a `for` loop, building a `main.exe`, running that in a container, then doing `docker kill -sTERM` on said container. The program prints `Got signal: SIGTERM`, then exits after the aforementioned timeout, as expected. Behavior before this patch is that the program gets no notification (and thus no output) but still exits after the timeout.
Fixes#7479
Change-Id: I2af79421cd484a0fbb9467bb7ddb5f0e8bc3610e
GitHub-Last-Rev: 9e05d631b5
GitHub-Pull-Request: golang/go#33311
Reviewed-on: https://go-review.googlesource.com/c/go/+/187739
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
The two methods act the same, so make their documentation similar so
that people don't think they act differently.
Change-Id: If224692ef50870faf855d789380a614d1e724132
Reviewed-on: https://go-review.googlesource.com/c/go/+/188137
Reviewed-by: Rob Pike <r@golang.org>
CL #163058 moves interpretation of platform-specific errors to the
syscall package. Package syscall errors implement an Is method which
os.IsPermission etc. consult. This results in an unintended semantic
change to the os package predicate functions: The following program
now prints 'true' where it used to print 'false':
package main
import "os"
type myError struct{ error }
func (e myError) Is(target error) bool { return target == os.ErrPermission }
func main() { println(os.IsPermission(myError{})) }
Change the os package error predicate functions to only examine syscall
errors, avoiding this semantic change.
This CL does retain one minor semantic change: On Plan9, os.IsPermission
used to return true for any error with text containing the string
"permission denied". It now only returns true for a syscall.ErrorString
containing that text.
Change-Id: I6b512b1de6ced46c2f1cc8d264fa2495ae7bf9f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/188817
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
It is unclear whether the current definition of os.IsTimeout is
desirable or not. Drop ErrTimeout for now so we can consider adding it
(or some other error) in a future release with a corrected definition.
Fixes#33411
Change-Id: I8b880da7d22afc343a08339eb5f0efd1075ecafe
Reviewed-on: https://go-review.googlesource.com/c/go/+/188758
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There's a race here with fork/exec, enable the close-on-exec flag
for the new file descriptor.
Fixes#33405
Change-Id: If95bae97a52b7026a930bb3427e47bae3b0032ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/188537
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As discussed in
https://github.com/golang/go/issues/32463#issuecomment-506833421
the classification of deadline-based timeouts as "temporary" errors is a
historical accident. I/O timeouts used to be duration-based, so they
really were temporary--retrying a timed-out operation could succeed. Now
that they're deadline-based, timeouts aren't temporary unless you reset
the deadline.
Drop ErrTemporary from Go 1.13, since its definition is wrong. We'll
consider putting it back in Go 1.14 with a clear definition and
deprecate net.OpError.Temporary.
Fixes#32463
Change-Id: I70cda664590d8872541e17409a5780da76920891
Reviewed-on: https://go-review.googlesource.com/c/go/+/188398
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Document that *os.File is subject to resource limits
for concurrent operations. We aren't documenting
a specific number of concurrent operations because that
number is OS/system dependent. This limit comes from:
internal/poll/fd_mutex.go
where we use 20 bits to count locks.
Fixes#32544
Change-Id: I7d305d4aaba5b2dbc6f1ab8c447117fde5e31a66
Reviewed-on: https://go-review.googlesource.com/c/go/+/181841
Reviewed-by: Rob Pike <r@golang.org>
The old code used ~/Library/Preferences, which is documented by
Apple as:
This directory contains app-specific preference files. You
should not create files in this directory yourself. Instead, use
the NSUserDefaults class or CFPreferences API to get and set
preference values for your app.
It looks like we missed everything after the first sentence; it's
definitely not the right choice for files that Go programs and users
should be touching directly.
Instead, use ~/Library/Application Support, which is documented as:
Use this directory to store all app data files except those
associated with the user’s documents. For example, you might use
this directory to store app-created data files, configuration
files, templates, or other fixed or modifiable resources that
are managed by the app. An app might use this directory to store
a modifiable copy of resources contained initially in the app’s
bundle. A game might use this directory to store new levels
purchased by the user and downloaded from a server.
This seems in line with what UserConfigDir is for, so use it.
The documentation quotes above are obtained from the surprisingly long
link below:
https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.htmlFixes#32475.
Change-Id: Ic27a6c92d76a5d7a4d4b8eac5cd8472f67a533a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/181177
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
On unix if exec.Command() is given both ExtraFiles and Ctty, and the
Ctty file descriptor overlaps the range of FDs intended for the child,
then cmd.Start() the ioctl(fd,TIOCSCTTY) call fails with an
"inappropriate ioctl for device" error.
When child file descriptors overlap the new child's ctty the ctty will
be closed in the fd shuffle before the TIOCSCTTY. Thus TIOCSCTTY is
used on one of the ExtraFiles rather than the intended Ctty file. Thus
the error.
exec.Command() callers can workaround this by ensuring the Ctty fd is
larger than any ExtraFiles destined for the child.
Fix this by doing the ctty ioctl before the fd shuffle.
Test for this issue by modifying TestTerminalSignal to use more
ExtraFiles. The test fails on linux and freebsd without this change's
syscall/*.go changes. Other platforms (e.g. darwin, aix, solaris) have
the same fd shuffle logic, so the same fix is applied to them. However,
I was only able to test on linux (32 and 64 bit) and freebsd (64 bit).
Manual runs of the test in https://golang.org/issue/29458 start passing
with this patch:
Before:
% /tmp/src/go/bin/go run t
successfully ran child process with ParentExtraFileFdNum=5, ChildExtraFileFd=6, ParentPtyFd=7
panic: failed to run child process with ParentExtraFileFdNum=10, ChildExtraFileFd=11, ParentPtyFd=11: fork/exec /bin/true: inappropriate ioctl for device
After:
% /tmp/src/go/bin/go run t
successfully ran child process with ParentExtraFileFdNum=5, ChildExtraFileFd=6, ParentPtyFd=7
successfully ran child process with ParentExtraFileFdNum=10, ChildExtraFileFd=11, ParentPtyFd=11
Fixes#29458
Change-Id: I99513de7b6073c7eb855f1eeb4d1f9dc0454ef8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/178919
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Shorten some of the longest tests that run during all.bash.
Removes 7r 50u 21s from all.bash.
After this change, all.bash is under 5 minutes again on my laptop.
For #26473.
Change-Id: Ie0460aa935808d65460408feaed210fbaa1d5d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/177559
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is CVE-2019-11888.
Previously, passing a nil environment but a non-nil token would result
in the new potentially unprivileged process inheriting the parent
potentially privileged environment, or would result in the new
potentially privileged process inheriting the parent potentially
unprivileged environment. Either way, it's bad. In the former case, it's
an infoleak. In the latter case, it's a possible EoP, since things like
PATH could be overwritten.
Not specifying an environment currently means, "use the existing
environment". This commit amends the behavior to be, "use the existing
environment of the token the process is being created for." The behavior
therefore stays the same when creating processes without specifying a
token. And it does the correct thing when creating processes when
specifying a token.
Fixes#32000
Change-Id: Ia57f6e89b97bdbaf7274d6a89c1d9948b6d40ef5
Reviewed-on: https://go-review.googlesource.com/c/go/+/176619
Run-TryBot: Jason Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Add Unwrap methods to types which wrap an underlying error:
"encodinc/csv".ParseError
"encoding/json".MarshalerError
"net/http".transportReadFromServerError
"net".OpError
"net".DNSConfigError
"net/url".Error
"os/exec".Error
"signal/internal/pty".PtyError
"text/template".ExecError
Add os.ErrTemporary. A case could be made for putting this error
value in package net, since no exported error types in package os
include a Temporary method. However, syscall errors returned from
the os package do include this method.
Add Is methods to error types with a Timeout or Temporary method,
making errors.Is(err, os.Err{Timeout,Temporary}) equivalent to
testing the corresponding method:
"context".DeadlineExceeded
"internal/poll".TimeoutError
"net".adrinfoErrno
"net".OpError
"net".DNSError
"net/http".httpError
"net/http".tlsHandshakeTimeoutError
"net/pipe".timeoutError
"net/url".Error
Updates #30322
Updates #29934
Change-Id: I409fb20c072ea39116ebfb8c7534d493483870dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/170037
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
Like GOOS=android which implies the "linux" build tag, GOOS=illumos
implies the "solaris" build tag. This lets the existing ecosystem of
packages still work on illumos, but still permits packages to start
differentiating between solaris and illumos.
Fixes#20603
Change-Id: I8f4eabf1a66060538dca15d7658c1fbc6c826622
Reviewed-on: https://go-review.googlesource.com/c/go/+/174457
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#20858
Change-Id: I45c397795426aaa276b20f5cbeb80270c95b920c
Reviewed-on: https://go-review.googlesource.com/c/go/+/174320
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Follow up CL 156379.
Updates #19093
Change-Id: I5ea3177fc5911d3af71cbb32584249e419e9d4a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/172937
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Though there is variation in the spelling of canceled,
cancellation is always spelled with a double l.
Reference: https://www.grammarly.com/blog/canceled-vs-cancelled/
Change-Id: I240f1a297776c8e27e74f3eca566d2bc4c856f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/170060
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The code to swap RemoveAllTestHook in and out in
TestRemoveAllWithMoreErrorThanReqSize was making a copy of the
RemoveAllTestHook pointer, then attempting to restore by loading from
the copy of that pointer. Since the two copies of the pointer aliased
the same address, the restore operation had no effect, and any
RemoveAll tests that happened to run after
TestRemoveAllWithMoreErrorThanReqSize would fail.
Fixes#31421
Change-Id: I7028475f5ceb3b0a2fa69d22af8d3379508c4531
Reviewed-on: https://go-review.googlesource.com/c/go/+/171777
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
golang.org/cl/121255 added close and re-open the directory when looping, prevent
us from missing some if previous iteration deleted files.
The CL introdued a bug. If we can not delete all entries in one request,
the looping never exits, causing RemoveAll hangs.
To fix that, simply discard the entries if we can not delete all of them
in one iteration, then continue reading entries and delete them.
Also make sure removeall_at return first error it encounters.
Fixes#29921
Change-Id: I8ec3a4c822d8d2d95d9f1ab71547879da395bc4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/171099
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The underlying system call tested by TestCredentialNoSetGroups
is blocked on Android.
Discovered while running all.bash from an Android device; the syscall
is only blocked in an app context.
Change-Id: I16fd2e64636a0958b0ec86820723c0577b8f8f24
Reviewed-on: https://go-review.googlesource.com/c/go/+/170945
Run-TryBot: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's not supported in an app context:
$ go test -short os
--- FAIL: TestChdirAndGetwd (0.00s)
os_test.go:1213: Open /: open /: permission denied
Change-Id: I56b951f925a50fd67715ee2f1de64951ee867e91
Reviewed-on: https://go-review.googlesource.com/c/go/+/170946
Run-TryBot: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Getdirentries is implemented with the __getdirentries64 function
in libSystem.dylib. That function works, but it's on Apple's
can't-be-used-in-an-app-store-application list.
Implement Getdirentries using the underlying fdopendir/readdir_r/closedir.
The simulation isn't faithful, and could be slow, but it should handle
common cases.
Don't use Getdirentries in the stdlib, use fdopendir/readdir_r/closedir
instead (via (*os.File).readdirnames).
Fixes#30933
Update #28984
RELNOTE=yes
Change-Id: Ia6b5d003e5bfe43ba54b1e1d9cfa792cc6511717
Reviewed-on: https://go-review.googlesource.com/c/go/+/168479
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Using os.UserHomeDir for user.HomeDir helps us deduplicate the
logic and keep the behavior consistent.
Also make os.UserHomeDir return "/sdcard" in android.
See: https://go-review.googlesource.com/c/go/+/37960/1/src/os/user/lookup_stubs.go#48Fixes#31070
Change-Id: I521bad050bc5761ecc5c0085501374d2cf8e6897
Reviewed-on: https://go-review.googlesource.com/c/go/+/169540
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
WriteAt use pwrite syscall on *nix or WriteFile on Windows.
On Linux/Windows, these system calls always write to end of file in
append mode, regardless of offset parameter.
It is hard (maybe impossible) to make WriteAt work portably.
Making WriteAt returns an error if file is opened in append mode, we
guarantee to get consistent behavior between platforms, also prevent
user from accidently corrupting their data.
Fixes#30716
Change-Id: If83d935a22a29eed2ff8fe53d13d0b4798aa2b81
Reviewed-on: https://go-review.googlesource.com/c/go/+/166578
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As proposed in Issue #29934, update errors produced by the os package to
work with errors.Is sentinel tests. For example,
errors.Is(err, os.ErrPermission) is equivalent to os.IsPermission(err)
with added unwrapping support.
Move the definition for os.ErrPermission and others into the syscall
package. Add an Is method to syscall.Errno and others. Add an Unwrap
method to os.PathError and others.
Updates #30322
Updates #29934
Change-Id: I95727d26c18a5354c720de316dff0bffc04dd926
Reviewed-on: https://go-review.googlesource.com/c/go/+/163058
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
Add an Unwrap method to PathError so it works with the errors.Is/As
functions.
Change-Id: Ia6171c0418584f3cd53ee99d97c687941a9e3109
Reviewed-on: https://go-review.googlesource.com/c/go/+/168097
Reviewed-by: Damien Neil <dneil@google.com>
