Release 3.2.2

Closes #7889.

CHANGELOG.rst

@@ -1,3 +1,20 @@
+fish 3.2.2 (released April 7, 2021)
+====================================
+
+This release of fish fixes a number of additional issues identified in the fish 3.2 series:
+
+- The command-not-found handler used suggestions from ``pacman`` on Arch Linux, but this caused major slowdowns on some systems and has been disabled (:issue:`7841`).
+- fish will no longer hang on exit if another process is in the foreground on macOS (:issue:`7901`).
+- Certain programs (such as ``lazygit``) could create situations where fish would not receive keystrokes correctly, but it is now more robust in these situations (:issue:`7853`).
+- Arguments longer than 1024 characters no longer trigger excessive CPU usage on macOS (:issue:`7837`).
+- fish builds correctly on macOS when using new versions of Xcode (:issue:`7838`).
+- Completions for ``aura`` (:issue:`7865`) and ``tshark`` (:issue:`7858`) should no longer produce errors.
+- Background jobs no longer interfere with syntax highlighting (a regression introduced in fish 3.2.1, :issue:`7842`).
+
+If you are upgrading from version 3.1.2 or before, please also review the release notes for 3.2.1 and 3.2.0 (included below).
+
+--------------
+
 fish 3.2.1 (released March 18, 2021)
 ====================================
 

cmake/Install.cmake

@@ -110,7 +110,7 @@ add_custom_command(OUTPUT fish.pc
     COMMAND printf "Version: " >> fish.pc
     COMMAND sed 's/FISH_BUILD_VERSION=//\;s/\"//g' ${FBVF} >> fish.pc
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
-    DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${FBVF} ${CMAKE_CURRENT_BINARY_DIR}/fish.pc.noversion)
+    DEPENDS CHECK-FISH-BUILD-VERSION-FILE ${CMAKE_CURRENT_BINARY_DIR}/fish.pc.noversion)
 
 add_custom_target(build_fish_pc ALL DEPENDS fish.pc)
 

share/completions/aura.fish

@@ -87,7 +87,7 @@ for condition in query sync
     complete -c aura -n $$condition -s s -l search -r -d 'Search packages for regexp'
 end
 
-for condition in abs aur
+for condition in aur
     complete -c aura -n $$condition -s a -l delmakedeps -d 'Remove packages only needed during installation'
     complete -c aura -n $$condition -s d -l deps -d 'View package dependencies'
     complete -c aura -n $$condition -s i -l info -d 'View package information'

share/completions/tshark.fish

@@ -5,14 +5,14 @@ __fish_complete_wireshark tshark
 function __fish_tshark_protocols
     set -l tok (commandline -ct | string collect)
     set -l tok_param (string replace -r -- '^-O' '' $tok)
-    command tshark -G protocols | while read -l -d \t name shortname identifier
+    command tshark -G protocols 2>/dev/null | while read -l -d \t name shortname identifier
         printf "%s%s\t%s\n" (string replace -r -- '(.+),[^,]*$' '$1,' $tok_param) $tok_no_comma $identifier $name
     end
 end
 
 complete -c tshark -s 2 -d 'Perform a two-pass analysis'
 # This is fairly expensive, but only done upon the user pressing tab.
-complete -c tshark -s e -d 'Add a field to the list of fields to display' -xa '(command tshark -G fields | awk -F\t \'{print $3"\t"$2}\')'
+complete -c tshark -s e -d 'Add a field to the list of fields to display' -xa '(command tshark -G fields 2> /dev/null | awk -F\t \'{print $3"\t"$2}\')'
 complete -c tshark -s E -d 'Set an option controlling the printing of fields' -xa '
 bom=y\t"Prepend output with the UTF-8 byte order mark"
 header=y\t"Print a list of the selected field names"
@@ -24,7 +24,7 @@ quote=\t"Set the quote character to use to surround fields d=\", s=\', n=no quot
 complete -c tshark -s F -d 'Set the output capture file format' -xa '(command tshark -F 2>| string replace -rf "\s+(\S+) - (.*)" \'$1\t$2\')'
 complete -c tshark -s G -d 'Print a glossary' -xa '(
 printf "help\tList available report types\n"
-command tshark -G help | string replace -rf "\s+-G (\S+)\s+(.*)" \'$1\t$2\'
+command tshark -G help 2>/dev/null | string replace -rf "\s+-G (\S+)\s+(.*)" \'$1\t$2\'
 )'
 complete -c tshark -s H -d 'Read a list of entries from a "hosts" file' -r
 complete -c tshark -s j -d 'Protocol match filter used for ek|json|jsonraw|pdml output file types' -x

share/functions/__fish_complete_wireshark.fish

@@ -6,15 +6,15 @@ end
 
 function __fish_wireshark_interface
     # no remote capture yet
-    command tshark -D | string replace -r ".*\. (\S+)\s*\(?([^)]*)\)?\$" '$1\t$2'
+    command tshark -D 2>/dev/null | string replace -r ".*\. (\S+)\s*\(?([^)]*)\)?\$" '$1\t$2'
 end
 
 function __fish_wireshark_protocol
-    command tshark -G protocols | awk -F\t '{print $3"\t"$1}'
+    command tshark -G protocols 2>/dev/null | awk -F\t '{print $3"\t"$1}'
 end
 
 function __fish_wireshark_heuristic
-    command tshark -G heuristic-decodes | awk -F\t '{print $2"\t"$1}'
+    command tshark -G heuristic-decodes 2>/dev/null | awk -F\t '{print $2"\t"$1}'
 end
 
 function __fish_tshark_name_resolving_flags
@@ -67,10 +67,10 @@ packets:\t"Switch to the next file after it contains N packets"'
     complete -c $shark -l list-time-stamp-types -d 'List time stamp types supported for the interface'
     complete -c $shark -s p -l no-promiscuous-mode -d "Don't put the interface into promiscuous mode"
     complete -c $shark -s s -l snapshot-length -d 'Set the default snapshot length in bytes to use when capturing live data' -x
-    complete -c $shark -l time-stamp-type -d "Change the interface's timestamp method" -xa '(__fish_wireshark_choices (command tshark --list-time-stamp-types))'
+    complete -c $shark -l time-stamp-type -d "Change the interface's timestamp method" -xa '(__fish_wireshark_choices (command tshark --list-time-stamp-types 2>/dev/null))'
     complete -c $shark -s v -l version -d 'Print the version and exit'
     complete -c $shark -s w -d 'Write raw packet data to the given file ("-" means stdout)' -r
-    complete -c $shark -s y -l linktype -d 'Set the data link type to use while capturing packets' -xa '(__fish_wireshark_choices (command tshark -L))'
+    complete -c $shark -s y -l linktype -d 'Set the data link type to use while capturing packets' -xa '(__fish_wireshark_choices (command tshark -L 2>/dev/null))'
 
     switch $shark
         case dumpcap tshark
@@ -81,7 +81,7 @@ packets:\t"Switch to the next file after it contains N packets"'
     switch $shark
         case wireshark tshark
             complete -c $shark -s C -d 'Run with the given configuration profile' -xa '(
-set -l folders (tshark -G folders | awk \'/Personal configuration/{ print $NF}\')/profiles/*
+set -l folders (tshark -G folders 2>/dev/null | awk \'/Personal configuration/{ print $NF}\')/profiles/*
 string match -r "[^/]*\\$" -- $folders)'
             complete -c $shark -s d -d 'Specify how a layer type should be dissected' -xa '(__fish_tshark_decode_as)'
             complete -c $shark -l enable-protocol -d 'Enable dissection of the given protocol' -xa '(__fish_wireshark_protocol)'
@@ -92,7 +92,7 @@ string match -r "[^/]*\\$" -- $folders)'
             complete -c $shark -s n -d 'Disable network object name resolution (hostname, TCP and UDP port names)'
             complete -c $shark -s N -d 'Turn on name resolution only for particular types of addresses and port numbers' -xa '( __fish_tshark_name_resolving_flags)'
             complete -c $shark -s o -d 'Override a preference value' -xa '(
-    command tshark -G defaultprefs | string replace -rf -- \'^#([a-z].*):.*\' \'$1:\')'
+    command tshark -G defaultprefs 2>/dev/null | string replace -rf -- \'^#([a-z].*):.*\' \'$1:\')'
             complete -c $shark -s r -l read-file -d 'Read packet data from the given file' -r
             complete -c $shark -s R -l read-filter -d 'Apply the given read filter' -x
             complete -c $shark -s t -d 'Set the format of the packet timestamp printed in summary lines' -xa '

share/functions/fish_command_not_found.fish

@@ -57,17 +57,18 @@ else if type -q pkgfile
             __fish_default_command_not_found_handler $argv[1]
         end
     end
-else if type -q pacman
-    function fish_command_not_found
-        set -l paths $argv[1]
-        # If we've not been given an absolute path, try $PATH as the starting point,
-        # otherwise pacman will try *every path*, and e.g. bash-completion
-        # isn't helpful.
-        string match -q '/*' -- $argv[1]; or set paths $PATH/$argv[1]
-        # Pacman only prints the path, so we still need to print the error.
-        __fish_default_command_not_found_handler $argv[1]
-        pacman -F $paths
-    end
+# pacman is too slow, see #7841.
+# else if type -q pacman
+#     function fish_command_not_found
+#         set -l paths $argv[1]
+#         # If we've not been given an absolute path, try $PATH as the starting point,
+#         # otherwise pacman will try *every path*, and e.g. bash-completion
+#         # isn't helpful.
+#         string match -q '/*' -- $argv[1]; or set paths $PATH/$argv[1]
+#         # Pacman only prints the path, so we still need to print the error.
+#         __fish_default_command_not_found_handler $argv[1]
+#         pacman -F $paths
+#     end
 else
     # Use standard fish command not found handler otherwise
     function fish_command_not_found --on-event fish_command_not_found

src/common.cpp

@@ -1789,12 +1789,15 @@ void save_term_foreground_process_group() {
 }
 
 void restore_term_foreground_process_group_for_exit() {
-    if (initial_fg_process_group != -1) {
-        // This is called during shutdown and from a signal handler. We don't bother to complain on
-        // failure because doing so is unlikely to be noticed.
-        // However we want this to fail if we are not the tty owner (#7060), so clear our SIGTTOU
-        // handler to allow it to fail properly. Note that we are about to exit.
-        (void)signal(SIGTTOU, SIG_DFL);
+    // We wish to restore the tty to the initial owner. There's two ways this can go wrong:
+    //  1. We may steal the tty from someone else (#7060).
+    //  2. The call to tcsetpgrp may deliver SIGSTOP to us, and we will not exit.
+    // Hanging on exit seems worse, so ensure that SIGTTOU is ignored so we do not get SIGSTOP.
+    // Note initial_fg_process_group == 0 is possible with Linux pid namespaces.
+    // This is called during shutdown and from a signal handler. We don't bother to complain on
+    // failure because doing so is unlikely to be noticed.
+    if (initial_fg_process_group > 0 && initial_fg_process_group != getpgrp()) {
+        (void)signal(SIGTTOU, SIG_IGN);
         (void)tcsetpgrp(STDIN_FILENO, initial_fg_process_group);
     }
 }

src/env_universal_common.cpp

@@ -1306,119 +1306,6 @@ static autoclose_fd_t make_fifo(const wchar_t *test_path, const wchar_t *suffix)
     return res;
 }
 
-// A universal notifier which uses SIGIO with a named pipe. This relies on the following behavior
-// which appears to work on Linux: if data becomes available on the pipe then all processes get
-// SIGIO even if the data is read (i.e. there is no race between SIGIO and another proc reading).
-//
-// A key difference between Linux and Mac is that on Linux SIGIO is only delivered if the pipe was
-// previously empty. That is, two successive writes with no reads will produce two SIGIOs on Mac but
-// only one on Linux.
-//
-// So, to post a notification, write anything to the pipe; if that would block drain the pipe and
-// try again.
-//
-// To receive a notification, watch for SIGIO on the read end, then read out the data to enable
-// SIGIO to be delivered again.
-class universal_notifier_sigio_t final : public universal_notifier_t {
-#ifdef SIGIO
-   public:
-    // Note we use a different suffix from universal_notifier_named_pipe_t to produce different
-    // FIFOs. This is because universal_notifier_named_pipe_t is level triggered while we are edge
-    // triggered. If they shared the same FIFO, then the named_pipe variant would keep firing
-    // forever.
-    explicit universal_notifier_sigio_t(const wchar_t *test_path)
-        : pipe_(try_make_pipe(test_path, L".notify")) {}
-
-    ~universal_notifier_sigio_t() = default;
-
-    void post_notification() override {
-        if (!pipe_.valid()) return;
-
-        // Write a byte to send SIGIO. If the pipe is full, read some and try again.
-        while (!write_1_byte()) {
-            if (errno == EINTR) {
-                continue;
-            } else if (errno == EAGAIN) {
-                // The pipe is full. Try once more.
-                drain_some();
-                write_1_byte();
-                break;
-            } else {
-                break;
-            }
-        }
-    }
-
-    bool poll() override {
-        if (sigio_count_ != signal_get_sigio_count()) {
-            // On Mac, SIGIO is generated on every write to the pipe.
-            // On Linux, it is generated only when the pipe goes from empty to non-empty.
-            // Read from the pipe so that SIGIO may be delivered again.
-            drain_some();
-            // We may have gotten another SIGIO because the pipe just became writable again.
-            // In particular BSD sends SIGIO on read even if there is no data to be read.
-            // Re-fetch the sigio count.
-            sigio_count_ = signal_get_sigio_count();
-            return true;
-        }
-        return false;
-    }
-
-   private:
-    // Construct a pipe for a given fifo path, arranging for SIGIO to be delivered.
-    // \return an invalid fd on failure.
-    static autoclose_fd_t try_make_pipe(const wchar_t *test_path, const wchar_t *suffix) {
-        autoclose_fd_t pipe = make_fifo(test_path, suffix);
-        if (!pipe.valid()) {
-            return autoclose_fd_t{};
-        }
-        // Note that O_ASYNC cannot be passed to open() (see Linux kernel bug #5993).
-        // We have to set it afterwards like so.
-        // Linux got support for O_ASYNC on fifos in 2.6 (released 2003). Treat its absence as a
-        // failure, but don't be noisy if this fails. Non-Linux platforms without O_ASYNC should use
-        // a different notifier strategy to avoid running into this.
-#ifdef O_ASYNC
-        if (fcntl(pipe.fd(), F_SETFL, O_NONBLOCK | O_ASYNC) == -1)
-#endif
-        {
-            FLOGF(uvar_file,
-                  _(L"fcntl(F_SETFL) failed, universal variable notifications disabled"));
-            return autoclose_fd_t{};
-        }
-        if (fcntl(pipe.fd(), F_SETOWN, getpid()) == -1) {
-            FLOGF(uvar_file,
-                  _(L"fcntl(F_SETOWN) failed, universal variable notifications disabled"));
-            return autoclose_fd_t{};
-        }
-        return pipe;
-    }
-
-    // Try writing a single byte to our pipe.
-    // \return true on success, false on error, in which case errno will be set.
-    bool write_1_byte() const {
-        assert(pipe_.valid() && "Invalid pipe");
-        char c = 0x42;
-        ssize_t amt = write(pipe_.fd(), &c, sizeof c);
-        return amt > 0;
-    }
-
-    // Read some data off of the pipe, discarding it.
-    void drain_some() const {
-        if (!pipe_.valid()) return;
-        char buff[256];
-        ignore_result(read(pipe_.fd(), buff, sizeof buff));
-    }
-
-    autoclose_fd_t pipe_{};
-    uint32_t sigio_count_{0};
-#else
-   public:
-    [[noreturn]] universal_notifier_sigio_t() {
-        DIE("universal_notifier_sigio_t cannot be used on this system");
-    }
-#endif
-};
-
 // Named-pipe based notifier. All clients open the same named pipe for reading and writing. The
 // pipe's readability status is a trigger to enter polling mode.
 //
@@ -1582,16 +1469,6 @@ universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_
     return strategy_notifyd;
 #elif defined(__CYGWIN__)
     return strategy_shmem_polling;
-#elif 0 && defined(SIGIO) && (defined(__APPLE__) || defined(__BSD__) || defined(__linux__))
-    // FIXME: The SIGIO notifier relies on an extremely specific interaction between signal handling and
-    // O_ASYNC writes, and doesn't currently work particularly well, so it's disabled.
-    // See discussion in #6585 and #7774 for examples of breakage.
-    //
-    // The SIGIO notifier does not yet work on WSL. See #7429
-    if (is_windows_subsystem_for_linux()) {
-        return strategy_named_pipe;
-    }
-    return strategy_sigio;
 #else
     return strategy_named_pipe;
 #endif
@@ -1612,9 +1489,6 @@ std::unique_ptr<universal_notifier_t> universal_notifier_t::new_notifier_for_str
         case strategy_shmem_polling: {
             return make_unique<universal_notifier_shmem_poller_t>();
         }
-        case strategy_sigio: {
-            return make_unique<universal_notifier_sigio_t>(test_path);
-        }
         case strategy_named_pipe: {
             return make_unique<universal_notifier_named_pipe_t>(test_path);
         }

src/env_universal_common.h

@@ -166,9 +166,6 @@ class universal_notifier_t {
         // Mac-specific notify(3) implementation.
         strategy_notifyd,
 
-        // Set up a fifo and then waits for SIGIO to be delivered on it.
-        strategy_sigio,
-
         // Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require
         // polling most of the time.
         strategy_named_pipe,

src/fish_test_helper.cpp

@@ -20,6 +20,26 @@ static void become_foreground_then_print_stderr() {
     fprintf(stderr, "become_foreground_then_print_stderr done\n");
 }
 
+static void nohup_wait() {
+    pid_t init_parent = getppid();
+    if (signal(SIGHUP, SIG_IGN)) {
+        perror("tcsetgrp");
+        exit(EXIT_FAILURE);
+    }
+    // Note: these silly close() calls are necessary to prevent our parent process (presumably fish)
+    // from getting stuck in the "E" state ("Trying to exit"). This appears to be a (kernel?) bug on
+    // macOS: the process is no longer running but is not a zombie either, and so cannot be reaped.
+    // It is unclear why closing these fds successfully works around this issue.
+    close(STDIN_FILENO);
+    close(STDOUT_FILENO);
+    close(STDERR_FILENO);
+    // To avoid leaving fish_test_helpers around, we exit once our parent changes, meaning the fish
+    // instance exited.
+    while (getppid() == init_parent) {
+        usleep(1000000 / 4);
+    }
+}
+
 static void report_foreground_loop() {
     int was_fg = -1;
     const auto grp = getpgrp();
@@ -146,6 +166,7 @@ struct fth_command_t {
 static fth_command_t s_commands[] = {
     {"become_foreground_then_print_stderr", become_foreground_then_print_stderr,
      "Claim the terminal (tcsetpgrp) and then print to stderr"},
+    {"nohup_wait", nohup_wait, "Ignore SIGHUP and just wait"},
     {"report_foreground", report_foreground, "Report to stderr whether we own the terminal"},
     {"report_foreground_loop", report_foreground_loop,
      "Continually report to stderr whether we own the terminal"},

src/fish_tests.cpp

@@ -195,6 +195,16 @@ static void popd() {
     env_stack_t::principal().set_pwd_from_getcwd();
 }
 
+// Helper to return a string whose length greatly exceeds PATH_MAX.
+wcstring get_overlong_path() {
+    wcstring longpath;
+    longpath.reserve(PATH_MAX * 2 + 10);
+    while (longpath.size() <= PATH_MAX * 2) {
+        longpath += L"/overlong";
+    }
+    return longpath;
+}
+
 // The odd formulation of these macros is to avoid "multiple unary operator" warnings from oclint
 // were we to use the more natural "if (!(e)) err(..." form. We have to do this because the rules
 // for the C preprocessor make it practically impossible to embed a comment in the body of a macro.
@@ -3977,8 +3987,7 @@ static void trigger_or_wait_for_notification(universal_notifier_t::notifier_stra
             usleep(40000);
             break;
         }
-        case universal_notifier_t::strategy_named_pipe:
-        case universal_notifier_t::strategy_sigio: {
+        case universal_notifier_t::strategy_named_pipe: {
             break;  // nothing required
         }
     }
@@ -3989,9 +3998,6 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
     std::unique_ptr<universal_notifier_t> notifiers[16];
     size_t notifier_count = sizeof notifiers / sizeof *notifiers;
 
-    // Set up SIGIO handler as needed.
-    signal_set_handlers(false);
-
     // Populate array of notifiers.
     for (size_t i = 0; i < notifier_count; i++) {
         notifiers[i] = universal_notifier_t::new_notifier_for_strategy(strategy, UVARS_TEST_PATH);
@@ -4041,12 +4047,6 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
 
     // Nobody should poll now.
     for (size_t i = 0; i < notifier_count; i++) {
-        // On BSD, SIGIO may be delivered by read() even if it returns EAGAIN;
-        // that is the polling itself may trigger a SIGIO. Therefore we poll twice.
-        if (strategy == universal_notifier_t::strategy_sigio) {
-            (void)poll_notifier(notifiers[i]);
-        }
-
         if (poll_notifier(notifiers[i])) {
             err(L"Universal variable notifier polled true after all changes, with strategy %d",
                 (int)strategy);
@@ -5336,6 +5336,13 @@ static void test_highlighting() {
         {L"# comment", highlight_role_t::comment},
     });
 
+    // Overlong paths don't crash (#7837).
+    const wcstring overlong = get_overlong_path();
+    highlight_tests.push_back({
+        {L"touch", highlight_role_t::command},
+        {overlong.c_str(), highlight_role_t::param},
+    });
+
     highlight_tests.push_back({
         {L"a", highlight_role_t::param},
         {L"=", highlight_role_t::operat, ns},
@@ -6184,6 +6191,47 @@ void test_normalize_path() {
     do_test(path_normalize_for_cd(L"/abc/def/", L"../ghi/..") == L"/abc/ghi/..");
 }
 
+void test_dirname_basename() {
+    say(L"Testing wdirname and wbasename");
+    const struct testcase_t {
+        const wchar_t *path;
+        const wchar_t *dir;
+        const wchar_t *base;
+    } testcases[] = {
+        {L"", L".", L"."},
+        {L"foo//", L".", L"foo"},
+        {L"foo//////", L".", L"foo"},
+        {L"/////foo", L"/", L"foo"},
+        {L"/////foo", L"/", L"foo"},
+        {L"//foo/////bar", L"//foo", L"bar"},
+        {L"foo/////bar", L"foo", L"bar"},
+
+        // Examples given in XPG4.2.
+        {L"/usr/lib", L"/usr", L"lib"},
+        {L"usr", L".", L"usr"},
+        {L"/", L"/", L"/"},
+        {L".", L".", L"."},
+        {L"..", L".", L".."},
+    };
+    for (const auto &tc : testcases) {
+        wcstring dir = wdirname(tc.path);
+        if (dir != tc.dir) {
+            err(L"Wrong dirname for \"%ls\": expected \"%ls\", got \"%ls\"", tc.path, tc.dir,
+                dir.c_str());
+        }
+        wcstring base = wbasename(tc.path);
+        if (base != tc.base) {
+            err(L"Wrong basename for \"%ls\": expected \"%ls\", got \"%ls\"", tc.path, tc.base,
+                base.c_str());
+        }
+    }
+    // Ensures strings which greatly exceed PATH_MAX still work (#7837).
+    wcstring longpath = get_overlong_path();
+    wcstring longpath_dir = longpath.substr(0, longpath.rfind(L'/'));
+    do_test(wdirname(longpath) == longpath_dir);
+    do_test(wbasename(longpath) == L"overlong");
+}
+
 static void test_topic_monitor() {
     say(L"Testing topic monitor");
     topic_monitor_t monitor;
@@ -6529,6 +6577,7 @@ int main(int argc, char **argv) {
     if (should_test_function("layout_cache")) test_layout_cache();
     if (should_test_function("prompt")) test_prompt_truncation();
     if (should_test_function("normalize")) test_normalize_path();
+    if (should_test_function("dirname")) test_dirname_basename();
     if (should_test_function("topics")) test_topic_monitor();
     if (should_test_function("topics")) test_topic_monitor_torture();
     if (should_test_function("pipes")) test_pipes();

src/highlight.cpp

@@ -918,6 +918,11 @@ void highlighter_t::color_as_argument(const ast::node_t &node) {
 static bool range_is_potential_path(const wcstring &src, const source_range_t &range,
                                     const operation_context_t &ctx,
                                     const wcstring &working_directory) {
+    // Skip strings exceeding PATH_MAX. See #7837.
+    // Note some paths may exceed PATH_MAX, but this is just for highlighting.
+    if (range.length > PATH_MAX) {
+        return false;
+    }
     // Get the node source, unescape it, and then pass it to is_potential_path along with the
     // working directory (as a one element list).
     bool result = false;

src/iothread.cpp

@@ -397,11 +397,18 @@ void iothread_perform_on_main(void_function_t &&func) {
 }
 
 bool make_detached_pthread(void *(*func)(void *), void *param) {
-    // The spawned thread inherits our signal mask. We don't want the thread to ever receive signals
-    // on the spawned thread, so temporarily block all signals, spawn the thread, and then restore
-    // it.
+    // The spawned thread inherits our signal mask. Temporarily block signals, spawn the thread, and
+    // then restore it. But we must not block SIGBUS, SIGFPE, SIGILL, or SIGSEGV; that's undefined
+    // (#7837). Conservatively don't try to mask SIGKILL or SIGSTOP either; that's ignored on Linux
+    // but maybe has an effect elsewhere.
     sigset_t new_set, saved_set;
     sigfillset(&new_set);
+    sigdelset(&new_set, SIGILL);   // bad jump
+    sigdelset(&new_set, SIGFPE);   // divide by zero
+    sigdelset(&new_set, SIGBUS);   // unaligned memory access
+    sigdelset(&new_set, SIGSEGV);  // bad memory access
+    sigdelset(&new_set, SIGSTOP);  // unblockable
+    sigdelset(&new_set, SIGKILL);  // unblockable
     DIE_ON_FAILURE(pthread_sigmask(SIG_BLOCK, &new_set, &saved_set));
 
     // Spawn a thread. If this fails, it means there's already a bunch of threads; it is very

src/reader.cpp

@@ -1342,7 +1342,8 @@ void reader_init() {
 
     // Set up our fixed terminal modes once,
     // so we don't get flow control just because we inherited it.
-    if (getpgrp() == tcgetpgrp(STDIN_FILENO)) {
+    if (is_interactive_session() &&
+        getpgrp() == tcgetpgrp(STDIN_FILENO)) {
         term_donate(/* quiet */ true);
     }
 

src/signal.cpp

@@ -207,11 +207,6 @@ void signal_clear_cancel() { s_cancellation_signal = 0; }
 
 int signal_check_cancel() { return s_cancellation_signal; }
 
-/// Number of SIGIO events.
-static volatile relaxed_atomic_t<uint32_t> s_sigio_count{0};
-
-uint32_t signal_get_sigio_count() { return s_sigio_count; }
-
 /// The single signal handler. By centralizing signal handling we ensure that we can never install
 /// the "wrong" signal handler (see #5969).
 static void fish_signal_handler(int sig, siginfo_t *info, void *context) {
@@ -276,14 +271,6 @@ static void fish_signal_handler(int sig, siginfo_t *info, void *context) {
             // test, to verify that we behave correctly when receiving lots of irrelevant signals.
             break;
 
-#if defined(SIGIO)
-        case SIGIO:
-            // An async FD became readable/writable/etc.
-            // Don't try to look at si_code, it is not set under BSD.
-            // Don't use ++ to avoid a CAS.
-            s_sigio_count = s_sigio_count + 1;
-            break;
-#endif
     }
     errno = saved_errno;
 }
@@ -366,11 +353,6 @@ void signal_set_handlers(bool interactive) {
     act.sa_flags = SA_SIGINFO;
     sigaction(SIGINT, &act, nullptr);
 
-    // Apply our SIGIO handler.
-    act.sa_sigaction = &fish_signal_handler;
-    act.sa_flags = SA_SIGINFO;
-    sigaction(SIGIO, &act, nullptr);
-
     // Whether or not we're interactive we want SIGCHLD to not interrupt restartable syscalls.
     act.sa_sigaction = &fish_signal_handler;
     act.sa_flags = SA_SIGINFO | SA_RESTART;

src/signal.h

@@ -42,10 +42,6 @@ int signal_check_cancel();
 /// In generaly this should only be done in interactive sessions.
 void signal_clear_cancel();
 
-/// \return a count of SIGIO signals.
-/// This is used by universal variables, and is a simple unsigned counter which wraps to 0.
-uint32_t signal_get_sigio_count();
-
 enum class topic_t : uint8_t;
 /// A sigint_detector_t can be used to check if a SIGINT (or SIGHUP) has been delivered.
 class sigchecker_t {

src/wutil.cpp

@@ -400,16 +400,51 @@ wcstring path_normalize_for_cd(const wcstring &wd, const wcstring &path) {
     return result;
 }
 
-wcstring wdirname(const wcstring &path) {
-    std::string tmp = wcs2string(path);
-    const char *narrow_res = dirname(&tmp[0]);
-    return str2wcstring(narrow_res);
+wcstring wdirname(wcstring path) {
+    // Do not use system-provided dirname (#7837).
+    // On Mac it's not thread safe, and will error for paths exceeding PATH_MAX.
+    // This follows OpenGroup dirname recipe.
+    // 1: Double-slash stays.
+    if (path == L"//") return path;
+
+    // 2: All slashes => return slash.
+    if (!path.empty() && path.find_first_not_of(L'/') == wcstring::npos) return L"/";
+
+    // 3: Trim trailing slashes.
+    while (!path.empty() && path.back() == L'/') path.pop_back();
+
+    // 4: No slashes left => return period.
+    size_t last_slash = path.rfind(L'/');
+    if (last_slash == wcstring::npos) return L".";
+
+    // 5: Remove trailing non-slashes.
+    path.erase(last_slash + 1, wcstring::npos);
+
+    // 6: Skip as permitted.
+    // 7: Remove trailing slashes again.
+    while (!path.empty() && path.back() == L'/') path.pop_back();
+
+    // 8: Empty => return slash.
+    if (path.empty()) path = L"/";
+    return path;
 }
 
-wcstring wbasename(const wcstring &path) {
-    std::string tmp = wcs2string(path);
-    char *narrow_res = basename(&tmp[0]);
-    return str2wcstring(narrow_res);
+wcstring wbasename(wcstring path) {
+    // This follows OpenGroup basename recipe.
+    // 1: empty => allowed to return ".". This is what system impls do.
+    if (path.empty()) return L".";
+
+    // 2: Skip as permitted.
+    // 3: All slashes => return slash.
+    if (!path.empty() && path.find_first_not_of(L'/') == wcstring::npos) return L"/";
+
+    // 4: Remove trailing slashes.
+    while (!path.empty() && path.back() == L'/') path.pop_back();
+
+    // 5: Remove up to and including last slash.
+    size_t last_slash = path.rfind(L'/');
+    if (last_slash != wcstring::npos) path.erase(0, last_slash + 1);
+    return path;
 }
 
 // Really init wgettext.

src/wutil.h

@@ -75,10 +75,10 @@ bool wreaddir_resolving(DIR *dir, const std::wstring &dir_path, wcstring &out_na
 bool wreaddir_for_dirs(DIR *dir, wcstring *out_name);
 
 /// Wide character version of dirname().
-std::wstring wdirname(const std::wstring &path);
+std::wstring wdirname(std::wstring path);
 
 /// Wide character version of basename().
-std::wstring wbasename(const std::wstring &path);
+std::wstring wbasename(std::wstring path);
 
 /// Wide character wrapper around the gettext function. For historic reasons, unlike the real
 /// gettext function, wgettext takes care of setting the correct domain, etc. using the textdomain

tests/pexpects/exit_nohang.py

@@ -0,0 +1,82 @@
+#!/usr/bin/env python3
+from pexpect_helper import SpawnedProc
+import subprocess
+import sys
+import signal
+import time
+import os
+
+sp = SpawnedProc()
+send, sendline, sleep, expect_prompt, expect_re = (
+    sp.send,
+    sp.sendline,
+    sp.sleep,
+    sp.expect_prompt,
+    sp.expect_re,
+)
+
+# Helper to print an error and exit.
+def error_and_exit(text):
+    keys = sp.colors()
+    print("{RED}Test failed: {NORMAL}{TEXT}".format(TEXT=text, **keys))
+    sys.exit(1)
+
+
+fish_pid = sp.spawn.pid
+
+# Launch fish_test_helper.
+expect_prompt()
+exe_path = os.environ.get("fish_test_helper")
+sp.sendline(exe_path + " nohup_wait")
+
+# We expect it to transfer tty ownership to fish_test_helper.
+sleep(0.1)
+tty_owner = os.tcgetpgrp(sp.spawn.child_fd)
+if fish_pid == tty_owner:
+    os.kill(fish_pid, signal.SIGKILL)
+    error_and_exit(
+        "Test failed: outer fish %d did not transfer tty owner to fish_test_helper"
+        % (fish_pid)
+    )
+
+
+# Now we are going to tell fish to exit.
+# It must not hang. But it might hang when trying to restore the tty.
+os.kill(fish_pid, signal.SIGTERM)
+
+# Loop a bit until the process exits (correct) or stops (incorrrect).
+# When it exits it should be due to the SIGTERM that we sent it.
+for i in range(10):
+    pid, status = os.waitpid(fish_pid, os.WUNTRACED | os.WNOHANG)
+    if pid == 0:
+        # No process ready yet, loop again.
+        sleep(0.1)
+    elif pid != fish_pid:
+        # Some other process exited (??)
+        os.kill(fish_pid, signal.SIGKILL)
+        error_and_exit(
+            "unexpected pid returned from waitpid. Got %d, expected %d"
+            % (pid, fish_pid)
+        )
+    elif os.WIFSTOPPED(status):
+        # Our pid stopped instead of exiting.
+        # Probably it stopped because of its tcsetpgrp call during exit.
+        # Kill it and report a failure.
+        os.kill(fish_pid, signal.SIGKILL)
+        error_and_exit("pid %d stopped instead of exiting on SIGTERM" % pid)
+    elif not os.WIFSIGNALED(status):
+        error_and_exit("pid %d did not signal-exit" % pid)
+    elif os.WTERMSIG(status) != signal.SIGTERM:
+        error_and_exit(
+            "pid %d signal-exited with %d instead of %d (SIGTERM)"
+            % (os.WTERMSIG(status), signal.SIGTERM)
+        )
+    else:
+        # Success!
+        sys.exit(0)
+else:
+    # Our loop completed without the process being returned.
+    error_and_exit("fish with pid %d hung after SIGTERM" % fish_pid)
+
+# Should never get here.
+error_and_exit("unknown, should be unreachable")