From 962b0f8b90884ceb68431acd8e148a6908650134 Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Mon, 17 May 2021 14:34:52 -0700
Subject: [PATCH] Pass $status to process-exit event handlers in all cases

Previously, an event handler would receive -1 if the process exited due
to a signal. Instead pass the same value as $status.
---
 CHANGELOG.rst            |  1 +
 src/fish_test_helper.cpp | 12 ++++++++----
 src/proc.cpp             |  4 ++--
 tests/checks/signal.fish | 12 +++++++++---
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 4e5ec6f7b..a5b8dda2c 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -26,6 +26,7 @@ Scripting improvements
 - Better errors when a command in a command substitution wasn't found or is not allowed.
 - ``fish_indent`` allows to write inline variable assignments on multiple lines (ending in a backslash), instead of joining them into one line (:issue:`7955`).
 - fish gained a ``--no-config`` option to disable reading config.fish. This applies both to the user's and the admin's config.fish (typically in /etc/fish/config.fish) and also sets $fish_function_path to just the functions shipped with fish and disables universal variables and history. (:issue:`7921`, :issue:`1256`).
+- ``process-exit`` event handlers now receive the same value as ``$status`` in all cases, instead of receiving -1 when the exit was due to a signal.
 
 Interactive improvements
 -------------------------
diff --git a/src/fish_test_helper.cpp b/src/fish_test_helper.cpp
index 94575754d..3ef6f99af 100644
--- a/src/fish_test_helper.cpp
+++ b/src/fish_test_helper.cpp
@@ -135,12 +135,10 @@ static void print_ignored_signals() {
 
 static void print_stop_cont() {
     signal(SIGTSTP, [](int) {
-        auto __attribute__((unused)) _ = write(STDOUT_FILENO, "SIGTSTP\n", strlen("SIGTSTP\n"));
+        (void)write(STDOUT_FILENO, "SIGTSTP\n", strlen("SIGTSTP\n"));
         kill(getpid(), SIGSTOP);
     });
-    signal(SIGCONT, [](int) {
-        auto __attribute__((unused)) _ = write(STDOUT_FILENO, "SIGCONT\n", strlen("SIGCONT\n"));
-    });
+    signal(SIGCONT, [](int) { (void)write(STDOUT_FILENO, "SIGCONT\n", strlen("SIGCONT\n")); });
     char buff[1];
     for (;;) {
         if (read(STDIN_FILENO, buff, sizeof buff) >= 0) {
@@ -149,6 +147,11 @@ static void print_stop_cont() {
     }
 }
 
+static void sigkill_self() {
+    kill(getpid(), SIGKILL);
+    abort();
+}
+
 static void show_help();
 
 /// A thing that fish_test_helper can do.
@@ -180,6 +183,7 @@ static fth_command_t s_commands[] = {
     {"print_ignored_signals", print_ignored_signals,
      "Print to stdout the name(s) of ignored signals"},
     {"print_stop_cont", print_stop_cont, "Print when we get SIGTSTP and SIGCONT, exiting on input"},
+    {"sigkill_self", sigkill_self, "Send SIGKILL to self"},
     {"help", show_help, "Print list of fish_test_helper commands"},
 };
 
diff --git a/src/proc.cpp b/src/proc.cpp
index d67f02ca6..c167b7e31 100644
--- a/src/proc.cpp
+++ b/src/proc.cpp
@@ -554,8 +554,8 @@ static bool try_clean_process_in_job(parser_t &parser, process_t *p, job_t *j,
 
     // Add an exit event if the process did not come from a job handler.
     if (!j->from_event_handler()) {
-        exit_events->push_back(proc_create_event(L"PROCESS_EXIT", event_type_t::exit, p->pid,
-                                                 s.normal_exited() ? s.exit_code() : -1));
+        exit_events->push_back(
+            proc_create_event(L"PROCESS_EXIT", event_type_t::exit, p->pid, s.status_value()));
     }
 
     // Ignore SIGPIPE. We issue it ourselves to the pipe writer when the pipe reader dies.
diff --git a/tests/checks/signal.fish b/tests/checks/signal.fish
index 9b49ceaca..d332f690a 100644
--- a/tests/checks/signal.fish
+++ b/tests/checks/signal.fish
@@ -1,4 +1,4 @@
-# RUN: %fish -C 'set -l fish %fish' %s
+# RUN: env fish_test_helper=%fish_test_helper %fish -C 'set -l fish %fish' %s
 
 $fish -c 'function main; exit 4; true; end; main'
 echo $status
@@ -47,13 +47,19 @@ command false | command true
 # CHECK: PROCESS_EXIT 0
 # CHECK: JOB_EXIT 0
 
+# Signals are reported correctly.
+# SIGKILL $status is 128 + 9 = 137
+$fish_test_helper sigkill_self
+# CHECK: PROCESS_EXIT 137
+# CHECK: JOB_EXIT 0
+
 function test_blocks
     block -l
-    command echo "This is the process whose exit event shuld be blocked"
+    command echo "This is the process whose exit event should be blocked"
     echo "This should come before the event handler"
 end
 test_blocks
-# CHECK: This is the process whose exit event shuld be blocked
+# CHECK: This is the process whose exit event should be blocked
 # CHECK: This should come before the event handler
 
 echo "Now event handler should have run"