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

ptmx.Read() blocks on Darwin #114

Closed
noahlt opened this issue Apr 14, 2021 · 2 comments
Closed

ptmx.Read() blocks on Darwin #114

noahlt opened this issue Apr 14, 2021 · 2 comments

Comments

@noahlt
Copy link

noahlt commented Apr 14, 2021

I'm writing a daemon that controls an instance of bash, using creack/pty to create the pseudoterminal, and writing commands to the pty to send them to bash and reading the output from the pty. On occasion, calling ptmx.Read() blocks the goroutine indefinitely.

I expect ptmx.Read() to block until the program on the tty ("slave") side of the pty writes something to read. Instead it seems to block forever. To test this, I wrote a program that tries to run echo $USER in a loop, which I have uploaded as a gist — occasionally the program runs the print on line 47 then hangs. The nondeterminism makes me suspect there's a race condition between my program and bash, but I still expected that ptmx.Read() would block until bash writes to the pty and then ptmx.Read() would unblock.

I also found that resizing my terminal (and sending sigwinch to the pty) causes the program to become unblocked. Not sure what to make of that.

Honestly I'm not sure whether this is a problem with pty_darwin.go or with my own code — so sorry if that's the case.

Environment details, in case they're helpful:

  • macOS 10.14.6
  • golang 1.16
  • creack/pty v1.1.11

@creack thanks for maintaining this project; it's excellent. Incidentally, let me know if you're open to doing some consulting work with pty.

@creack
Copy link
Owner

creack commented Apr 15, 2021

The issue is most likely in the readUntil function, could you provide it as well? I suspect it simply reads more than what you expect so the following ptmx.Read doesn't have anything more to read. Sending a SIGWINCH results in new activity on the slave side, so the read unblocks.

I did manage to reproduce the random blocking behavior using this naive implementation of readUntil:

const PromptChar = '\x19'

func readUntil(r io.Reader, sep, buf []byte) int {
        n, err := r.Read(buf)
        if err != nil {
                panic(err)
        }
        idx := bytes.Index(buf[:n], sep)
        if idx < 0 {
                log.Printf("sep %q not found in %q\r\n", sep, buf[:n])
        }
        return idx
}

A simple fix would be to use bufio.Reader. Here is a slightly modified version of the provided snippet which doesn't block: https://gist.github.com/creack/68448647d726fbfd4fb7bd04ae5358dd

Using a pty in a daemon is usually a bad idea, it is better to use the stdlib's exec.Command directly, which avoids having to deal with all prompt management / echo'd commands and terminal overhead.
There are some use-cases where it can be useful and maybe indeed you have such use-case, but I'd recommend checking and make sure that is the case.

In a daemon, stdin would be closed, so the whole logic around SIGWINCH and Raw mode would be superfluous and should be removed.

@creack
Copy link
Owner

creack commented May 29, 2021

Closing as it is not related to the lib. Feel free to reach out if you need more help. Thank you for the sponsorship.

@creack creack closed this as completed May 29, 2021
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

3 participants
@noahlt @creack and others