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

Is there a way to unblock io.Copy(ptmx, os.Stdin) after exiting the spawned shell #188

Closed
joshi4 opened this issue Jan 31, 2024 · 4 comments

Comments

@joshi4
Copy link

joshi4 commented Jan 31, 2024

Thanks for creating this package!

I'm running into an issue where pty doesn't seem to play nice with https://github.com/charmbracelet/bubbletea.

I observe that bubbletea programs hang until I press a key ( after exiting the spawned shell by typing exit or ctrl-d)

I'm following the example in the README closely and I think this particular line below is causing the problem.

        // Copy stdin to the pty and the pty to stdout.
        // NOTE: The goroutine will keep reading until the next keystroke before returning.
        go func() { _, _ = io.Copy(ptmx, os.Stdin) }()

The need for an extra keystroke to unblock that go-routine seems to be the main issue.

Is there any way to exit the go routine that copies from stdin to ptmx when ptmx closes?

@joshi4
Copy link
Author

joshi4 commented Jan 31, 2024

I had cross posted this question in the bubbletea discord too and their reply lead me to
cancelreader, which is exactly what I was looking for.

Since this issue is non deterministic, I will test it a bit more and follow up here if cancelReader does indeed fix everything.

@creack
Copy link
Owner

creack commented Jan 31, 2024

Can you specify your OS? I suspect it is OSX, right? We could set the read in non-block mode, but Golang has a bug on OSX resulting in this using 100% CPU. But it would also behave as you expect.

An alternative would be to use select (2) syscall (not go's select keyword) or to watch for SIGIO before read, that way you can be sure the Read call will not block, and it should work everywhere.

That is a bit outside the scope of the library though, as it deals more about I/Os than TTYs.

@creack
Copy link
Owner

creack commented Jan 31, 2024

For reference: golang/go#22099

joshi4 added a commit to getsavvyinc/savvy-cli that referenced this issue Feb 1, 2024
The only way to io.Copy(dst, os.Stdin) returns in raw mode
is when io.Copy tries to read a character from stdin and find that dst
is no longer valid.

For our use here, that presents a problem.

Users expect the input to close when they exit the shell. We can't get
an extra input here to unblock io.Copy when it is reading from stdin.

Enter, cancelreader.

We wrap os.Stdin with a cancel reader that can be preempted from another
go-routine by calling Cancel() on the cancel reader.

The implementation of the cancel reader is platform dependent, but the
pkg should work across linux, osx and windows.

@creack also points out an alternative way to ensure we don't block on
io.Copy reads here: creack/pty#188 (comment)
joshi4 added a commit to getsavvyinc/savvy-cli that referenced this issue Feb 1, 2024
The only way to io.Copy(dst, os.Stdin) returns in raw mode
is when io.Copy tries to read a character from stdin and find that dst
is no longer valid.

For our use here, that presents a problem.

Users expect the input to close when they exit the shell. We can't get
an extra input here to unblock io.Copy when it is reading from stdin.

Enter, cancelreader.

We wrap os.Stdin with a cancel reader that can be preempted from another
go-routine by calling Cancel() on the cancel reader.

The implementation of the cancel reader is platform dependent, but the
pkg should work across linux, osx and windows.

@creack also points out an alternative way to ensure we don't block on
io.Copy reads here: creack/pty#188 (comment)
@joshi4
Copy link
Author

joshi4 commented Feb 1, 2024

verified that using the cancelreader works for now.

Thanks for the suggestion of using sigio too. If I run into corner cases with the cancel reader, I will give that a shot.

@joshi4 joshi4 closed this as completed Feb 1, 2024
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