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

io.Copy blocked after closing #88

Closed
eudore opened this issue Dec 12, 2019 · 2 comments
Closed

io.Copy blocked after closing #88

eudore opened this issue Dec 12, 2019 · 2 comments

Comments

@eudore
Copy link

eudore commented Dec 12, 2019

After ptmx is closed, io.Copy will not read and return an error.

example:

package main

import (
	"os/exec"
	"io"
	"fmt"
	"time"
	"github.com/creack/pty"
)

type w struct{}

func main() {
	cmd := exec.Command("bash")
	ptmx, err := pty.Start(cmd)
	if err != nil {
		panic(err)
	}
	go func() {
		time.Sleep(1* time.Second)
		fmt.Println("timeout")
		ptmx.Close()
	}()
	fmt.Println(io.Copy(&w{}, ptmx))
	fmt.Println("stop")
}

func (*w) Write(p []byte) (int,error) {
	return len(p), nil
}

example2:

func main() {
	cmd := exec.Command("bash")
	ptmx, err := pty.Start(cmd)
	if err != nil {
		panic(err)
	}
	go func() {
		time.Sleep(1 * time.Second)
		fmt.Println("timeout")
		ptmx.Close()
	}()
	body := make([]byte, 2048)
	for {
		n, err := ptmx.Read(body)
		if err != nil {
			fmt.Println(err)
			break
		}
		fmt.Println(body[0:n], err)
	}
	fmt.Println("stop")
}
@creack
Copy link
Owner

creack commented Dec 17, 2019

This seems like expected behavior.

Could you describe your use case and expected result more in detail?

exec.Command.Start forks the process, resulting the the duplication of the ptmx/tty pair file discriptor. When the ptmx gets closed in the parent, it doesn't close the child one.

@eudore
Copy link
Author

eudore commented Dec 20, 2019

Usually src or dest can exit after calling Close method io.Copy.
I hope that ptmx.Close will close the bidirectional io.Copy, so how should I close the pty accurately?

Scenario: websocket reads and writes a bash terminal.

The NewWebscoketConn function will convert ws.Conn into an rwc object, and then io.Copy uses rwc and ptmx to exchange read and write data streams.

The object returned by the NewWebscoketConn function implements the io.ReadWriteCloser interface, which encapsulates the reading and writing of websocket data.

import (
	"github.com/creack/pty"
	"github.com/gobwas/ws"
)
func handlebash(w http.ResponseWriter, r *http.Request) {
	conn, _, _, err := ws.UpgradeHTTP(r, w)
	if err != nil {
		return
	}

	cmd := exec.Command("bash")
	ptmx, err := pty.Start(cmd)
	if err != nil {
		return
	}
	log.Println("start bash")

	wconn := NewWebscoketConn(conn)
	go func() {
		log.Println(io.Copy(ptmx, wconn))
		log.Println("stop wconn", wconn.Close())
		ptmx.Write([]byte(" "))
		ptmx.Close()
	}()
	go func() {
		log.Println(io.Copy(wconn, ptmx))
		log.Println("stop ptmx", ptmx.Close())
		// wconn.Write([]byte("\r\n"))
		wconn.Close()
	}()
}

Initially, after wconn.Close (), Write triggers err to exit io.Copy.

With your prompt, I modified an implementation that uses ctx to close cmd to close ptmx.

	ctx, cannel := context.WithCancel(context.Background())
	cmd := exec.CommandContext(ctx, "bash")
	go func() {
		log.Println(io.Copy(ptmx, wconn))
		log.Println("stop wconn", wconn.Close())
		cannel()
	}()
	go func() {
		log.Println(io.Copy(wconn, ptmx))
		log.Println("stop ptmx")
	}()

@eudore eudore closed this as completed Jan 12, 2020
sio added a commit to sio/creack-pty that referenced this issue Apr 28, 2023
When compiled with go older than 1.12 creack/pty will not include a fix
for blocking Read() and will be prone to data races - but at least it will work

For more information see issues: creack#88 creack#114 creack#156 creack#162
sio added a commit to sio/creack-pty that referenced this issue Apr 28, 2023
When compiled with go older than 1.12 creack/pty will not include a fix
for blocking Read() and will be prone to data races - but at least it will work

For more information see issues: creack#88 creack#114 creack#156 creack#162
aymanbagabas pushed a commit to aymanbagabas/pty that referenced this issue Sep 22, 2023
When compiled with go older than 1.12 creack/pty will not include a fix
for blocking Read() and will be prone to data races - but at least it will work

For more information see issues: creack#88 creack#114 creack#156 creack#162
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

No branches or pull requests

2 participants