Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-26 04:43:10 +00:00
Code Issues Projects Releases Packages Wiki Activity

Prevent disown from directly removing jobs

Browse source 
This prevents the `disown` builtin from directly removing jobs out of
the jobs list to prevent sanity issues, as `disown` may be called within
the context of a subjob (e.g. in a function or block) in which case the
parent job might not yet be done with the reference to the child job.

Instead, a flag is set and the parser removes the job from the list only
after the entire execution chain has completed.

Closes #5720.
This commit is contained in:
Mahmoud Al-Qudsi 2019-04-09 23:29:58 -05:00
parent f1b261388a
commit 394623bf08
4 changed files with 20 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/builtin_disown.cpp
View file

@ -17,7 +17,7 @@
/// Helper for builtin_disown.
static int disown_job(const wchar_t *cmd, parser_t &parser, io_streams_t &streams, job_t *j) {
if (j == 0) {
if (j == nullptr) {
streams.err.append_format(_(L"%ls: Unknown job '%ls'\n"), L"bg");
builtin_print_error_trailer(parser, streams.err, cmd);
return STATUS_INVALID_ARGS;
@ -31,17 +31,13 @@ static int disown_job(const wchar_t *cmd, parser_t &parser, io_streams_t &stream
streams.err.append_format(fmt, cmd, j->job_id, j->command_wcstr());
}
for (auto itr = jobs().begin(); itr != jobs().end(); ++itr) {
auto job = itr->get();
if (job == j) {
pid_t pgid = j->pgid;
add_disowned_pgid(pgid);
jobs().erase(itr);
return STATUS_CMD_OK;
}
}
// We cannot directly remove the job from the jobs() list as `disown` might be called
// within the context of a subjob which will cause the parent job to crash in exec_job().
// Instead, we set a flag and the parser removes the job from the jobs list later.
j->set_flag(job_flag_t::PENDING_REMOVAL, true);
add_disowned_pgid(j->pgid);
return STATUS_CMD_ERROR;
return STATUS_CMD_OK;
}
/// Builtin for removing jobs from the job list.

4
src/builtin_jobs.cpp
View file

@ -175,7 +175,7 @@ int builtin_jobs(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
if (print_last) {
// Ignore unconstructed jobs, i.e. ourself.
for (const auto &j : jobs()) {
if (j->is_constructed() && !j->is_completed()) {
if (j->is_visible()) {
builtin_jobs_print(j.get(), mode, !streams.out_is_redirected, streams);
return STATUS_CMD_ERROR;
}
@ -217,7 +217,7 @@ int builtin_jobs(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
} else {
for (const auto &j : jobs()) {
// Ignore unconstructed jobs, i.e. ourself.
if (j->is_constructed() && !j->is_completed()) {
if (j->is_visible()) {
builtin_jobs_print(j.get(), mode, !found && !streams.out_is_redirected, streams);
found = true;
}

5
src/parse_execution.cpp
View file

@ -1269,6 +1269,11 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t<g::job> jo
remove_job(job.get());
}
// This job was disowned during its own execution or the execution of its subjobs
if (job->get_flag(job_flag_t::PENDING_REMOVAL)) {
remove_job(job.get());
}
// Only external commands require a new fishd barrier.
if (job_contained_external_command) {
set_proc_had_barrier(false);

6
src/proc.h
View file

@ -257,6 +257,10 @@ enum class job_flag_t {
JOB_CONTROL,
/// Whether the job wants to own the terminal when in the foreground.
TERMINAL,
/// This job needs to be removed from the list of jobs for one reason or another (killed,
/// completed, disowned, etc). This flag is set rather than directly manipulating the jobs
/// list.
PENDING_REMOVAL,
JOB_FLAG_COUNT
};
@ -396,6 +400,8 @@ class job_t {
bool is_completed() const;
/// The job is in a stopped state
bool is_stopped() const;
/// The job is OK to be externally visible, e.g. to the user via `jobs`
bool is_visible() const { return !is_completed() && is_constructed() && !get_flag(job_flag_t::PENDING_REMOVAL); };
/// \return the parent job, or nullptr.
const std::shared_ptr<job_t> get_parent() const { return parent_job; }