linux: fix a hang if there are no reads from the tty #1623
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
marked as a draft, as I need to test it more |
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
6 times, most recently
from
4555a83 to
a8a67b4
Compare
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
a8a67b4 to
61c2d7d
Compare
|
this PR fixes the problem as expected 🎉 |
|
/approve |
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
61c2d7d to
c1c2271
Compare
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
c1c2271 to
50b60e2
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
50b60e2 to
67d862e
Compare
kwilczynski
reviewed
Dec 15, 2024
| int ret, container_exit_code = 0, last_process; | ||
| cleanup_close int terminal_fd_from = -1; | ||
| cleanup_close int terminal_fd_to = -1; | ||
| const size_t max_events = 10; |
There was a problem hiding this comment.
Would it make sense to make this a definition with an added comment about what it's for?
There was a problem hiding this comment.
not sure, I can add a comment if you feel we need it, but this is an arbitrary limit defined only to avoid using the same constant multiple times.
There was a problem hiding this comment.
No worries. This is fine, too. A #define is just a habit.
commit 1832c17 introduced the change. Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
67d862e to
c687e11
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
c687e11 to
66be626
Compare
Signed-off-by: Giuseppe Scrivano <[email protected]>
avoid a "crun exec" hang when the the other end of the terminal stopped reading. That happened because `copy_fd_to_fd` tried to write everything that it has received from the source fd, so it would hang the current process. Prevent that using non blocking file descriptors and using epoll to detect when the file descriptor is available for write. Fixes: https://issues.redhat.com/browse/OCPBUGS-45632 Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
fix-hang-on-unresponsive-tty
branch
from
66be626 to
8b972be
Compare
|
@saschagrunert @haircommander PTAL. It is ready to merge (assuming a happy CI) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
avoid a "crun exec" hang when the the other end of the terminal stopped reading.
That happened because
copy_fd_to_fdtried to write everything that it has received from the source fd, so it would hang the current process. Prevent that using non blocking file descriptors and using epoll to detect when the file descriptor is available for write.Fixes: https://issues.redhat.com/browse/OCPBUGS-45632