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

Setctty set but Ctty not valid in child with Go +be08e10b3b #96

Closed
myitcv opened this issue May 5, 2020 · 3 comments · Fixed by #97
Closed

Setctty set but Ctty not valid in child with Go +be08e10b3b #96

myitcv opened this issue May 5, 2020 · 3 comments · Fixed by #97

Comments

@myitcv
Copy link

myitcv commented May 5, 2020

Steps to reproduce:

go version
go run .

-- go.mod --
module mod.com

go 1.15

require (
        github.com/creack/pty v1.1.9
        golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3 // indirect
)
-- main.go --
package main

import (
        "io"
        "io/ioutil"
        "os/exec"

        "github.com/creack/pty"
)

func main() {
        cmd := exec.Command("vim")
        thepty, err := pty.Start(cmd)
        if err != nil {
                panic(err)
        }
        io.Copy(ioutil.Discard, thepty)
}

What did you expect to see?

No error

What did you see instead?

Error:

fork/exec /home/myitcv/dev/vim/src/vim: Setctty set but Ctty not valid in child

Bisected to this change: https://go-review.googlesource.com/c/go/+/231638/

FYI @ianlancetaylor @neild @mvdan

@myitcv
Copy link
Author

myitcv commented May 5, 2020

Previous discussion in golang/go#29458

@creack
Copy link
Owner

creack commented May 5, 2020

reproduced. Investigating.

@ianlancetaylor
Copy link

Note copied from golang/go#29458:

If tty is going to be open in the child process, then it must have a file descriptor in the child process. When using os/exec.Cmd as the creack/pty code does, tty must be in Stdin or Stdout or Stderr or ExtraFiles. If it isn't in any of those, then it will be closed when the child process runs. If it is in one of those, that gives you the file descriptor number that you should use in Ctty.

Right now the creack code is setting Ctty to the parent's file descriptor number. That is working (assuming that it is working) by accident. The correct fix is to ensure that tty is passed to the child somewhere, and set Ctty to the appropriate descriptor number. If it is possible for all of Stdin, Stdout, Stderr to be set, the simplest approach would be to always add tty to ExtraFiles, and set Ctty to 3. That will work for old versions of Go and also for the upcoming 1.15 release.

@creack creack closed this as completed in #97 May 7, 2020
myitcv added a commit to govim/govim that referenced this issue May 7, 2020
myitcv added a commit to govim/govim that referenced this issue May 11, 2020
pmorjan added a commit to gotoz/runq that referenced this issue Jun 16, 2020
The upcoming Go 1.15 compiler requires an update of the creack/pty package
or runq-exec fails with:
"fork/exec /sbin/nsenter: Setctty set but Ctty not valid in child"

See creack/pty#96 for details.

Signed-off-by: Peter Morjan <peter.morjan@de.ibm.com>
btwiuse pushed a commit to btwiuse/k0s that referenced this issue Jul 18, 2020
muryliang added a commit to muryliang/ssh that referenced this issue Sep 4, 2020
nickysemenza added a commit to nickysemenza/air that referenced this issue Sep 5, 2020
Pulls in fix for creack/pty#96
Was causing `failed to build, error: fork/exec /bin/sh: Setctty set but
Ctty not valid in child`
nickysemenza added a commit to nickysemenza/air that referenced this issue Sep 5, 2020
Pulls in fix for creack/pty#96
Was causing `failed to build, error: fork/exec /bin/sh: Setctty set but
Ctty not valid in child`
nickysemenza added a commit to nickysemenza/air that referenced this issue Sep 5, 2020
Pulls in fix for creack/pty#96
Was causing `failed to build, error: fork/exec /bin/sh: Setctty set but
Ctty not valid in child`
42wim added a commit to 42wim/ipsetd that referenced this issue Sep 28, 2020
mdrohmann added a commit to ActiveState/termtest that referenced this issue Oct 20, 2020
This idea for this fix is copied from creack/pty#96
anthonyfok added a commit to anthonyfok/gexpect that referenced this issue Oct 30, 2020
Fixes "Setctty set but Ctty not valid in child with Go" error
with Go 1.15 and up.  See creack/pty#96
anthonyfok added a commit to anthonyfok/gexpect that referenced this issue Oct 30, 2020
Fixes "Setctty set but Ctty not valid in child with Go" error
with Go 1.15 and up.  See creack/pty#96
anthonyfok added a commit to anthonyfok/gexpect that referenced this issue Oct 30, 2020
Fixes "Setctty set but Ctty not valid in child" error with Go 1.15 and up.
See creack/pty#96 for details.
jakewright added a commit to jakewright/home-automation that referenced this issue Nov 15, 2020
clrpackages pushed a commit to clearlinux-pkgs/docker that referenced this issue Nov 19, 2020
Need to patch creack/pty for this bug:
creack/pty#96

Signed-off-by: Mark D Horn <mark.d.horn@intel.com>
clrpackages pushed a commit to clearlinux-pkgs/kata-runtime that referenced this issue Nov 19, 2020
clrpackages pushed a commit to clearlinux-pkgs/kata-shim that referenced this issue Nov 19, 2020
Need to patch creack/pty for this bug:
creack/pty#96

Signed-off-by: Mark D Horn <mark.d.horn@intel.com>
antonmedv pushed a commit to antonmedv/watch that referenced this issue Dec 9, 2020
Cheaterman added a commit to Cheaterman/sampctl that referenced this issue Aug 15, 2021
Cheaterman added a commit to Cheaterman/sampctl that referenced this issue Aug 15, 2021
ADRFranklin added a commit to Southclaws/sampctl that referenced this issue Aug 15, 2021
gopherbot pushed a commit to golang/build that referenced this issue Sep 10, 2021
gomote ssh connections immediately close with a "Connection closed"
error. The coordinator logs indicate an error in our call to pty.Start,
which is used to relay the tty between ssh connections. When the
coordinator was updated to Go 1.16 from Go 1.13 in July, it likely
caused this issue (see creack/pty#96).

Fixes golang/go#48329

Change-Id: Ieec5d2220c6c5ef4c53de594acbdd598a1bafb6d
Reviewed-on: https://go-review.googlesource.com/c/build/+/349092
Trust: Alexander Rakoczy <alex@golang.org>
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
btwiuse pushed a commit to btwiuse/ameniicsa that referenced this issue Sep 11, 2021
ADRFranklin pushed a commit to Southclaws/sampctl that referenced this issue Jan 2, 2022
ADRFranklin pushed a commit to Southclaws/sampctl that referenced this issue Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants