builtins to only acquire terminal if owned by their pgroup

Fix #5133 changed builtins to acquire the terminal, but this regressed
caused fish to be stopped when running in background via `sudo fish`.
Fix this by only acquiring the terminal if the terminal was owned by the
builtin's pgroup.

Fixes #5147
This commit is contained in:
ridiculousfish
2018-08-18 16:56:01 -07:00
parent cbcabf6d00
commit d9f34147c3
5 changed files with 9 additions and 9 deletions

View File

@@ -486,7 +486,7 @@ static const wcstring_list_t help_builtins({L"for", L"while", L"function", L"if"
static bool cmd_needs_help(const wchar_t *cmd) { return contains(help_builtins, cmd); }
/// Execute a builtin command
int builtin_run(parser_t &parser, wchar_t **argv, io_streams_t &streams) {
int builtin_run(parser_t &parser, int job_pgid, wchar_t **argv, io_streams_t &streams) {
UNUSED(parser);
UNUSED(streams);
if (argv == NULL || argv[0] == NULL) return STATUS_INVALID_ARGS;
@@ -503,7 +503,7 @@ int builtin_run(parser_t &parser, wchar_t **argv, io_streams_t &streams) {
// 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;
pid_t pgroup_to_restore = grab_tty ? terminal_acquire_before_builtin(job_pgid) : -1;
int ret = data->func(parser, streams, argv);
if (pgroup_to_restore >= 0) {
tcsetpgrp(STDIN_FILENO, pgroup_to_restore);

View File

@@ -88,7 +88,7 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION };
void builtin_init();
bool builtin_exists(const wcstring &cmd);
int builtin_run(parser_t &parser, wchar_t **argv, io_streams_t &streams);
int builtin_run(parser_t &parser, int job_pgrp, wchar_t **argv, io_streams_t &streams);
wcstring_list_t builtin_get_names();
void builtin_get_names(std::vector<completion_t> *list);

View File

@@ -484,7 +484,7 @@ static bool exec_internal_builtin_proc(parser_t &parser, job_t *j, process_t *p,
j->set_flag(JOB_FOREGROUND, false);
// Note this call may block for a long time, while the builtin performs I/O.
p->status = builtin_run(parser, p->get_argv(), streams);
p->status = builtin_run(parser, j->pgid, p->get_argv(), streams);
// Restore the fg flag, which is temporarily set to false during builtin
// execution so as not to confuse some job-handling builtins.

View File

@@ -870,10 +870,10 @@ bool terminal_give_to_job(const job_t *j, bool cont) {
return true;
}
pid_t terminal_acquire_before_builtin() {
pid_t terminal_acquire_before_builtin(int job_pgid) {
pid_t selfpid = getpid();
pid_t current_owner = tcgetpgrp(STDIN_FILENO);
if (current_owner >= 0 && current_owner != selfpid) {
if (current_owner >= 0 && current_owner != selfpid && current_owner == job_pgid) {
if (tcsetpgrp(STDIN_FILENO, selfpid) == 0) {
return current_owner;
}

View File

@@ -374,6 +374,6 @@ pid_t proc_wait_any();
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();
/// Given that we are about to run a builtin, acquire the terminal if it is owned by the given job.
/// Returns the pid to restore after running the builtin, or -1 if there is no pid to restore.
pid_t terminal_acquire_before_builtin(int job_pgid);