diff --git a/src/fish.cpp b/src/fish.cpp index 3e25c5bd6..3bd1742e6 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -474,7 +474,7 @@ int main(int argc, char **argv) { // TODO: The generic process-exit event is useless and unused. // Remove this in future. - proc_fire_event(L"PROCESS_EXIT", event_type_t::exit, getpid(), exit_status); + event_fire(proc_create_event(L"PROCESS_EXIT", event_type_t::exit, getpid(), exit_status)); // Trigger any exit handlers. wcstring_list_t event_args = {to_string(exit_status)}; diff --git a/src/proc.cpp b/src/proc.cpp index fccfabb66..072e948dd 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -436,14 +436,14 @@ static void print_job_status(const job_t *j, job_status_t status) { outp.flush_to(STDOUT_FILENO); } -void proc_fire_event(const wchar_t *msg, event_type_t type, pid_t pid, int status) { +event_t proc_create_event(const wchar_t *msg, event_type_t type, pid_t pid, int status) { event_t event{type}; event.desc.param1.pid = pid; event.arguments.push_back(msg); event.arguments.push_back(to_string(pid)); event.arguments.push_back(to_string(status)); - event_fire(event); + return event; } /// Remove all disowned jobs whose job chain is fully constructed (that is, do not erase disowned @@ -461,16 +461,21 @@ void remove_disowned_jobs(job_list_t &jobs) { } /// Given a a process in a job, print the status message for the process as appropriate, and then -/// mark the status code so we don't print again. \return true if we printed a status message, false -/// if not. -static bool try_clean_process_in_job(process_t *p, job_t *j, bool only_one_job) { +/// mark the status code so we don't print again. Populate any events into \p exit_events. +/// \return true if we printed a status message, false if not. +static bool try_clean_process_in_job(process_t *p, job_t *j, std::vector *exit_events, + bool only_one_job) { if (!p->completed || !p->pid) { return false; } auto s = p->status; - // Ignore signal SIGPIPE.We issue it ourselves to the pipe writer when the pipe reader - // dies. + + // Add an exit event. + exit_events->push_back(proc_create_event(L"PROCESS_EXIT", event_type_t::exit, p->pid, + s.normal_exited() ? s.exit_code() : -1)); + + // Ignore SIGPIPE. We issue it ourselves to the pipe writer when the pipe reader dies. if (!s.signal_exited() || s.signal_code() == SIGPIPE) { return false; } @@ -566,6 +571,12 @@ static bool process_clean_after_marking(bool allow_interactive) { // If we ever drop the `static bool locked` above, this should be changed to a local or // thread-local vector instead of a static vector. It is only static to reduce heap allocations. static std::vector> erase_list; + + // Accumulate exit events into a new list, which we fire after the list manipulation is + // complete. + std::vector exit_events; + + // Print status messages for completed or stopped jobs. const bool only_one_job = jobs().size() == 1; for (const auto &j : jobs()) { // Skip unconstructed jobs. @@ -582,7 +593,7 @@ static bool process_clean_after_marking(bool allow_interactive) { // Note this may print the message on behalf of the job, affecting the result of // job_wants_message(). for (process_ptr_t &p : j->processes) { - if (try_clean_process_in_job(p.get(), j.get(), only_one_job)) { + if (try_clean_process_in_job(p.get(), j.get(), &exit_events, only_one_job)) { printed = true; } } @@ -594,6 +605,16 @@ static bool process_clean_after_marking(bool allow_interactive) { printed = true; } + // Prepare events for completed jobs. + if (j->is_completed()) { + if (j->pgid != INVALID_PID) { + exit_events.push_back( + proc_create_event(L"JOB_EXIT", event_type_t::exit, -j->pgid, 0)); + } + exit_events.push_back( + proc_create_event(L"JOB_EXIT", event_type_t::job_exit, j->job_id, 0)); + } + // Remove us from the job list if we're complete. if (j->is_completed()) { erase_list.push_back(j); @@ -626,9 +647,9 @@ static bool process_clean_after_marking(bool allow_interactive) { } } - // Only now notify any listeners of the job exit, so that the contract isn't violated - for (const auto &j : erase_list) { - proc_fire_event(L"JOB_EXIT", event_type_t::job_exit, j->job_id, 0); + // Post pending exit events. + for (const auto &evt : exit_events) { + event_fire(evt); } erase_list.clear(); diff --git a/src/proc.h b/src/proc.h index e62633d35..e74610e93 100644 --- a/src/proc.h +++ b/src/proc.h @@ -502,9 +502,8 @@ void proc_update_jiffies(); /// job is in the foreground, that every process is in a valid state, etc. void proc_sanity_check(); -/// Send a process/job exit event notification. This function is a convenience wrapper around -/// event_fire(). -void proc_fire_event(const wchar_t *msg, event_type_t type, pid_t pid, int status); +/// Create a process/job exit event notification. +event_t proc_create_event(const wchar_t *msg, event_type_t type, pid_t pid, int status); /// Initializations. void proc_init(); diff --git a/tests/jobs.in b/tests/jobs.in index 7be84be4b..7132953e4 100644 --- a/tests/jobs.in +++ b/tests/jobs.in @@ -21,3 +21,12 @@ function foo jobs -c end foo + +# Verify we observe job exit events +sleep 1 & +set sleep_job $last_pid +function sleep_done_$sleep_job --on-job-exit $sleep_job + /bin/echo "sleep is done" + functions --erase sleep_done_$sleep_job +end +sleep 2 diff --git a/tests/jobs.out b/tests/jobs.out index 53ff0b574..62e4d97d5 100644 --- a/tests/jobs.out +++ b/tests/jobs.out @@ -8,3 +8,4 @@ sleep sleep Command sleep +sleep is done diff --git a/tests/signal.out b/tests/signal.out index f2cd4188c..10aa9b5f3 100644 --- a/tests/signal.out +++ b/tests/signal.out @@ -1,8 +1,17 @@ ALRM received command false: +PROCESS_EXIT 1 +JOB_EXIT 0 command true: +PROCESS_EXIT 0 +JOB_EXIT 0 command false | true: +PROCESS_EXIT 1 +PROCESS_EXIT 0 +JOB_EXIT 0 This is the process whose exit event shuld be blocked This should come before the event handler +PROCESS_EXIT 0 +JOB_EXIT 0 Now event handler should have run PROCESS_EXIT 0