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.

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

cmd/exec: Wait hangs on DragonflyBSD if child has a PTY that has been written to #56132

Open
prattmic opened this issue Oct 10, 2022 · 1 comment
Open

cmd/exec: Wait hangs on DragonflyBSD if child has a PTY that has been written to #56132

prattmic opened this issue Oct 10, 2022 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Dragonfly
Milestone
Backlog

Comments

@prattmic
Copy link
Member

What version of Go are you using (go version)?

tip at 6a9aaf1

Does this issue reproduce with the latest release?

Yes, 1.19.2

What operating system and processor architecture are you using (go env)?

dragonfly-amd64. Specifically, the dragonfly-amd64-622 builder/gomote.

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="dragonfly"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="dragonfly"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/tmp/workdir/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/tmp/workdir/go/pkg/tool/dragonfly_amd64"
GOVCS=""
GOVERSION="devel gomote.XXXXX"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/workdir/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2229047134=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run TestDragonfly from CL 441916.

$ git fetch https://go.googlesource.com/go refs/changes/16/441916/1 && git checkout FETCH_HEAD
$ ./make.bash
$ ../bin/go test -run=TestDragonfly -v os/signal

This test:

  1. Creates psuedo-terminal parent and child FDs (with posix_openpt et al via cgo in os/signal/internal/pty).
  2. Starts a child process which inherits the child end of the PTY.
    1. Starting a new session (Setsid), process group (Setpgid), setting the controlling terminal (Setctty), and setting foreground (Foreground) don't seem to make a difference either way, so the repro skips them.
  3. The child immediately exits (reading from the PTY does not make a difference).
  4. The parent writes a byte to the parent end of the PTY (if nothing is written hang does not occur).
  5. The parent waits on the child with cmd.Wait.

What did you expect to see?

Test passes.

Note that TryBots passed on all other Unix OSes.

What did you see instead?

Test hangs, and eventually times out.

$ ../bin/go test -run=TestDragonfly -v -count=1 os/signal
=== RUN   TestDragonfly
    signal_cgo_test.go:301: Waiting for exit...
child exiting
<hung>

Specifically, the child exits (CALL exit(0) according to ktrace), but the parent wait6 call never returns. Full ktrace dump is attached here.

This feels like an OS bug, though I attempted to recreate this in C (dragonfly.c in CL 441916) and was not able to reproduce. That said, there are some differences from Go, most notably that both the parent and child are multithreaded in Go.

cc @golang/dragonfly
@prattmic prattmic added OS-Dragonfly NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 10, 2022
@prattmic prattmic added this to the Backlog milestone Oct 10, 2022
@gopherbot
Copy link

Change https://go.dev/cl/440220 mentions this issue: os/signal: rewrite TestTerminalSignal without bash

gopherbot pushed a commit that referenced this issue Oct 14, 2022
@prattmic @gopherbot
os/signal: rewrite TestTerminalSignal without bash
a8ca70f 
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>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@prattmic @romaindoumenc
os/signal: rewrite TestTerminalSignal without bash
a6f5b3c 
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
(golang#56132).

Updates golang#56132.
Fixes golang#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>
@bcmills bcmills mentioned this issue Dec 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Dragonfly
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@prattmic @gopherbot