Skip to content

Fix reuse of pty slave devices #2449

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.

Sign up for GitHub

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

Open
cgull wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix reuse of pty slave devices #2449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e781ef0
Copy ish-lld.lldb to build dir
cgull Aug 31, 2024
6b0856c
Support pty slave reuse and behave more like Linux
cgull Aug 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions fs/pty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ static int pty_slave_open(struct tty *tty) {
return _EIO;
if (tty->pty.locked)
return _EIO;
// If userland's reference count on the pty slave is now 1, clear
// hang_up on the pty master. But the session leader may have a
// reference, and the pty master always has a reference.
if (tty->refcount - 1 == (tty->session ? 2 : 1)) {

@tbodt tbodt Aug 31, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this check a bit unsettling in the last PR but figured it could be fixed later. Seeing it again, I'm thinking much the same thing... Ideally we would define explicitly which references count for this and which ones don't, and with that knowledge the likely solution is to create a second refcount field for tracking just those.

tty->pty.other->hung_up = false;

@tbodt tbodt Aug 31, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs locking, right?

}
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions fs/tty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ static ssize_t tty_read(struct fd *fd, void *buf, size_t bufsize) {
lock(&tty->lock);
if (tty->hung_up || pty_is_half_closed_master(tty)) {
unlock(&pids_lock);
err = -1;

@tbodt tbodt Aug 31, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an errno constant

goto error;
}

Expand Down
4 changes: 2 additions & 2 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ endif

subdir('tools')

gdb_scripts = ['ish-gdb.gdb']
foreach script : gdb_scripts
debugger_scripts = ['ish-gdb.gdb', 'ish-lldb.lldb']
foreach script : debugger_scripts
custom_target(script,
output: script, input: script,
command: ['ln', '-sf', '@INPUT@', '@OUTPUT@'],
Expand Down