-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
[3.10] bpo-26228: Fix pty EOF handling (GH-12049) #27732
Conversation
On non-Linux POSIX platforms, like FreeBSD or macOS, the FD used to read a forked PTY may signal its exit not by raising an error but by sending empty data to the read syscall. This case wasn't handled, leading to hanging `pty.spawn` calls. Co-authored-by: Reilly Tucker Siemens <reilly@tuckersiemens.com> Co-authored-by: Łukasz Langa <lukasz@langa.pl> (cherry picked from commit 81ab8db) Co-authored-by: Zephyr Shannon <geoffpshannon@gmail.com>
@RadicalZephyr and @ambv: Status check is done, and it's a success ✅ . |
@RadicalZephyr and @ambv: Status check is done, and it's a success ✅ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
15 similar comments
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@pablogsal, this is a bugfix we should have done a long time ago. If you feel messing with PTY at RC stage is too risky, we can merge this post-3.10.0. Both here, and on the original PR, I ran with This backport has an outstanding In any case, we have to have this PR in at some point as it's one of the changes that will allow running -R: -uall on macOS. |
@@ -0,0 +1 @@ | |||
pty.spawn no longer hangs on FreeBSD, OS X, and Solaris. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should say "macOS" but I have a follow-up PR for doc updates anyway so I'll fix that there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the change and I think is safe enough. Also we have another RC so we can still test it to some degree
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a success ✅ . |
@RadicalZephyr and @ambv: Status check is done, and it's a pending ❌ . |
@RadicalZephyr and @ambv: Status check is done, and it's a success ✅ . |
On non-Linux POSIX platforms, like FreeBSD or macOS,
the FD used to read a forked PTY may signal its exit not
by raising an error but by sending empty data to the read
syscall. This case wasn't handled, leading to hanging
pty.spawn
calls.Co-authored-by: Reilly Tucker Siemens reilly@tuckersiemens.com
Co-authored-by: Łukasz Langa lukasz@langa.pl
(cherry picked from commit 81ab8db)
Co-authored-by: Zephyr Shannon geoffpshannon@gmail.com
https://bugs.python.org/issue26228