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

tty: internal mode state does not account for changes outside of libuv #1292

Open
wavexx opened this issue Apr 7, 2017 · 12 comments
Open

tty: internal mode state does not account for changes outside of libuv #1292

wavexx opened this issue Apr 7, 2017 · 12 comments
Labels

Comments

@wavexx
Copy link
Contributor

wavexx commented Apr 7, 2017

When changing the tty mode (via uv_tty_set_mode), we keep an internal "mode" state to avoid doing ioctls again. I see this is used to restore the original terminal mode, but it introduces some issues.

When using a pty to control a slave process, the slave might change the mode on our back. I have this problem on a couple of occasions, where I need to force a call to cfmakeraw() but I need to toggle the internal state (by changing the tty mode to NORMAL and then IO again) to actually do it.

I'd be tempted to add a "force" parameter to always set the requested mode, but maybe there's a better solution?

@bnoordhuis
Copy link
Member

Are you referring to the if (mode == tty_handle->mode) return 0 check? I'd be okay with removing that provided it's replaced with a comment explaining why libuv doesn't do an early exit - and assuming it doesn't cause regressions, of course.

@wavexx
Copy link
Contributor Author

wavexx commented Apr 7, 2017 via email

@wavexx
Copy link
Contributor Author

wavexx commented Apr 9, 2017

See PR #1293

After reviewing the code, I really don't like how uv_tty_reset_mode() works. I didn't change its behavior, but it works on the assumption that you only change one TTY. If you're working with multiple PTYs, uv_tty_reset_mode resets the tty which was first modified, which is senseless. Changing the behavior without changing the ABI though is tough.

I hope it's more clear in the docs now.

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 10, 2019
@vtjnash
Copy link
Member

vtjnash commented Aug 12, 2019

In progress/discussion in #1293?

@stale stale bot removed the stale label Aug 12, 2019
@wavexx
Copy link
Contributor Author

wavexx commented Aug 12, 2019

Didn't touch tty code in a while, I resorted to an external process helper.
I'm working on completely different stuff currently, but I still consider this an issue if one wants to do proper tty manipulation.

@stale
Copy link

stale bot commented Nov 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 10, 2019
@stale stale bot closed this as completed Nov 17, 2019
@vtjnash vtjnash reopened this Nov 17, 2019
@stale stale bot removed the stale label Nov 17, 2019
@stale
Copy link

stale bot commented Jan 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 13, 2020
@stale stale bot closed this as completed May 13, 2020
@vtjnash vtjnash reopened this May 13, 2020
@stale stale bot removed the stale label May 13, 2020
@stale
Copy link

stale bot commented Jun 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 3, 2020
@stale stale bot closed this as completed Oct 1, 2020
@vtjnash vtjnash reopened this Oct 1, 2020
@stale stale bot removed the stale label Oct 1, 2020
@wavexx
Copy link
Contributor Author

wavexx commented Oct 1, 2020 via email

@stale
Copy link

stale bot commented Oct 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 23, 2020
@stale stale bot closed this as completed Mar 20, 2021
@vtjnash vtjnash reopened this Mar 20, 2021
@stale stale bot removed the stale label Mar 20, 2021
@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants