Forgot to cc @kr

When you call pty.Start, are you setting the Stdin, Stdout, and Stderr fields?

I don't quite see how github.com/kr/pty works today. It uses os.OpenFile to open the tty, but that will mark the file descriptor as close-on-exec. Maybe it worked before because before CL 178919 we dup'ed the ctty to stdin/stdout/stderr before calling TIOCSCTTY, and so the ctty designation carried over. After the CL we call TIOCSCTTY first, and maybe that doesn't survive the later dup'ing. Or something.

It might help to compare an strace -f of your program before and after CL 178919, to see what changed.

It might fix the problem if github.com/kr/pty were changed to use syscall.Open rather than os.OpenFile for the pty, so that it is not marked close-on-exec.

Edit: this analysis was flawed because I was not looking at the same version of the pty package that @myitcv was using.

CC @gthelen

I'm happy to help with changes to github.com/kr/pty if necessary, especially if there are steps we can follow to reproduce this.

Also cc @creack.

Happy to help, however, I cloned and installed 103b5b6 on linux/amd64 (ubuntu 16.04) and have not been able to reproduce.

I tried vim, bash, emacs, tmux using the sample code form kr/pty readme without a problem.
I tried tried govim, but I think I'm doing something wrong as it didn't start anything in a pty..

Could you provide a more detailed way to reproduce?

Using syscall.Open instead of os.OpenFile could be done as well.

For reference, here is what I tested with, from https://github.com/kr/pty/blob/master/README.md#shell

package main

import (
        "io"
        "log"
        "os"
        "os/exec"
        "os/signal"
        "syscall"

        "github.com/kr/pty"
        "golang.org/x/crypto/ssh/terminal"
)

func test() error {
        // Create arbitrary command.
        c := exec.Command("vim")
        // Tried this and variations.
        c.SysProcAttr = &syscall.SysProcAttr{
                Setsid:     true,
                Setctty:    true,
        }

        // Start the command with a pty.
        ptmx, err := pty.Start(c)
        if err != nil {
                return err
        }
        // Make sure to close the pty at the end.
        defer func() { _ = ptmx.Close() }() // Best effort.

        // Handle pty size.
        ch := make(chan os.Signal, 1)
        signal.Notify(ch, syscall.SIGWINCH)
        go func() {
                for range ch {
                        if err := pty.InheritSize(os.Stdin, ptmx); err != nil {
                                log.Printf("error resizing pty: %s", err)
                        }
                }
        }()
        ch <- syscall.SIGWINCH // Initial resize.

        // Set stdin in raw mode.
        oldState, err := terminal.MakeRaw(int(os.Stdin.Fd()))
        if err != nil {
                panic(err)
        }
        defer func() { _ = terminal.Restore(int(os.Stdin.Fd()), oldState) }() // Best effort.

        // Copy stdin to the pty and the pty to stdout.
        go func() { _, _ = io.Copy(ptmx, os.Stdin) }()
        _, _ = io.Copy(os.Stdout, ptmx)

        return nil
}

func main() {
        if err := test(); err != nil {
                log.Fatal(err)
        }
}

I'm also happy to help, but have no problems running creack's vim workload (#32527 (comment)) with golang 103b5b6 on linux/amd64.

Thanks very much for all the offers of help debugging this.

I've not actually managed to create a repro that shows exactly the same symptoms, but I do have a small example that fails pre/post that CL in a slightly different but perhaps related way. That follows at the end of this comment.

In what follows I will use:

• "pre" to refer to a53b465, the commit immediately prior to CL 178919
• "post" to refer to 103b5b6, the commit that resulted from the merge of CL 178919

I've also tried to keep this issue a bit shorter by putting details in separate repo.

Vim version details for all my tests, attempted repros etc: 00 - vim version

Original issue

Per @ianlancetaylor I've used strace -f on the original problem. This was triggered as part of a test, so I've compiled the test binary and run strace -f on that; the issue remains (but I mention that for full disclosure on the setup).

$ go test -c -o test.prog
$ strace -f ./test.prog -test.run TestScripts/scripts/autocmd

• 01 - pre strace -f
• 02 - post strace -f

In answer to your specific question, @ianlancetaylor :

When you call pty.Start, are you setting the Stdin, Stdout, and Stderr fields?

No; simply setting a working directory and environment.

Please excuse the code in testdriver; it's horrendous and needs a rewrite.

Small example

The simple example in https://github.com/myitcvscratch/ptyrepro also breaks as a result of CL 178919, but not in the same way.

Pre:

$ go run main.go

No output; i.e. no panic.

Post:

panic: fork/exec /home/myitcv/usr/vim/bin/vim: operation not permitted

goroutine 1 [running]:
main.main()
        /home/myitcv/dev/ptyrepro/main.go:15 +0xdb
exit status 2

Here is the strace -f output for both:

• 01 - pre strace -f
• 02 - post strace -f

Again, happy to help provide any further details that help to track this down.

Thanks for the strace listings and the small repro.

When I go run your repro, it downloads pty 1.1.4. That is missing change kr/pty@b6e1bdd which sets the Ctty field in SysProcAttr. The effect is that it always sets the controlling terminal to file descriptor 0. Before CL 178919, that worked for your test case because setting the ctty ran after dup'ing the pty to descriptor 0. After CL 178919, it fails, because before the dup'ing descriptor 0 is not a pty.

If I change the go.mod file in your small repro to

require github.com/kr/pty b6e1bdd4a4f88614e0c6e5e8089c7abed98aae17

then your program succeeds.

@ianlancetaylor - thank you for taking the time to look into this, very much appreciated.

I can confirm that with kr/pty@b6e1bdd both my repro and the original issue are resolved.

I will close this issue and open an issue in https://github.com/kr/pty about cutting a new release.