Acquire tty if interactive when running builtins

When running a builtin, if we are an interactive shell and stdin is a tty,
then acquire ownership of the terminal via tcgetpgrp() before running the
builtin, and set it back after.

Fixes #4540
This commit is contained in:
ridiculousfish 2018-08-04 17:32:04 -07:00
parent c0a332743f
commit fa66ac8d8c
4 changed files with 32 additions and 1 deletions

View file

@ -500,7 +500,15 @@ int builtin_run(parser_t &parser, wchar_t **argv, io_streams_t &streams) {
} }
if (const builtin_data_t *data = builtin_lookup(argv[0])) { if (const builtin_data_t *data = builtin_lookup(argv[0])) {
return data->func(parser, streams, argv); // If we are interactive, save the foreground pgroup and restore it after in case the
// builtin needs to read from the terminal. See #4540.
bool grab_tty = is_interactive_session && isatty(streams.stdin_fd);
pid_t pgroup_to_restore = grab_tty ? terminal_acquire_before_builtin() : -1;
int ret = data->func(parser, streams, argv);
if (pgroup_to_restore >= 0) {
tcsetpgrp(STDIN_FILENO, pgroup_to_restore);
}
return ret;
} }
debug(0, UNKNOWN_BUILTIN_ERR_MSG, argv[0]); debug(0, UNKNOWN_BUILTIN_ERR_MSG, argv[0]);

View file

@ -870,6 +870,17 @@ bool terminal_give_to_job(const job_t *j, bool cont) {
return true; return true;
} }
pid_t terminal_acquire_before_builtin() {
pid_t selfpid = getpid();
pid_t current_owner = tcgetpgrp(STDIN_FILENO);
if (current_owner >= 0 && current_owner != selfpid) {
if (tcsetpgrp(STDIN_FILENO, selfpid) == 0) {
return current_owner;
}
}
return -1;
}
/// Returns control of the terminal to the shell, and saves the terminal attribute state to the job, /// Returns control of the terminal to the shell, and saves the terminal attribute state to the job,
/// so that we can restore the terminal ownership to the job at a later time. /// so that we can restore the terminal ownership to the job at a later time.
static bool terminal_return_from_job(job_t *j) { static bool terminal_return_from_job(job_t *j) {

View file

@ -373,3 +373,7 @@ pid_t proc_wait_any();
#endif #endif
bool terminal_give_to_job(const job_t *j, bool cont); bool terminal_give_to_job(const job_t *j, bool cont);
/// Given that we are about to run a builtin, acquire the terminal if we do not own it. Returns the
/// pid to restore after running the builtin, or -1 if there is no pid to restore.
pid_t terminal_acquire_before_builtin();

View file

@ -90,6 +90,14 @@ expect_prompt
expect_marker 7 expect_marker 7
print_var_contents foo print_var_contents foo
# Verify we don't hang on `read | cat`. See #4540.
send_line "read | cat"
expect_read_prompt
send_line -h "bar\r_marker 4540"
expect_prompt
expect_marker 4540
# ========== # ==========
# The fix for issue #2007 initially introduced a problem when using a function # The fix for issue #2007 initially introduced a problem when using a function
# to read from /dev/stdin when that is associated with the tty. These tests # to read from /dev/stdin when that is associated with the tty. These tests