merge branch 'bug-537' and branch 'bug-read-ctrlC'. This should fix both #537 and #516

5 files changed, 81 insertions, 8 deletions

diff --git a/builtin.cpp b/builtin.cpp
index 646db2b0..e8254533 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -2399,6 +2399,7 @@ static int builtin_read(parser_t &parser, wchar_t **argv)
         }
         /* No autosuggestions in builtin_read */
         reader_set_allow_autosuggesting(false);
+        reader_set_exit_on_interrupt(true);
 
         reader_set_buffer(commandline, wcslen(commandline));
         proc_push_interactive(1);
diff --git a/input.cpp b/input.cpp
index 5c178be8..1c60b93c 100644
--- a/input.cpp
+++ b/input.cpp
@@ -292,7 +292,7 @@ static int interrupt_handler()
     /*
       Tell the reader an event occured
     */
-    if (reader_interrupted())
+    if (reader_reading_interrupted())
     {
         /*
           Return 3, i.e. the character read by a Control-C.
@@ -507,7 +507,7 @@ wint_t input_readch()
     /*
        Clear the interrupted flag
        */
-    reader_interrupted();
+    reader_reset_interrupted();
 
     /*
        Search for sequence in mapping tables
diff --git a/reader.cpp b/reader.cpp
index 411615f5..99a9c251 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -45,6 +45,7 @@ commence.
 #include <unistd.h>
 #include <wctype.h>
 #include <stack>
+#include <pthread.h>
 
 #if HAVE_NCURSES_H
 #include <ncurses.h>
@@ -180,8 +181,11 @@ commence.
  */
 #define SEARCH_FORWARD 1
 
-/* Any time the contents of a buffer changes, we update the generation count. This allows for our background highlighting thread to notice it and skip doing work that it would otherwise have to do. */
-static unsigned int s_generation_count;
+/* Any time the contents of a buffer changes, we update the generation count. This allows for our background highlighting thread to notice it and skip doing work that it would otherwise have to do. This variable should really be of some kind of interlocked or atomic type that guarantees we're not reading stale cache values. With C++11 we should use atomics, but until then volatile should work as well, at least on x86.*/
+static volatile unsigned int s_generation_count;
+
+/* This pthreads generation count is set when an autosuggestion background thread starts up, so it can easily check if the work it is doing is no longer useful. */
+static pthread_key_t generation_count_key;
 
 /* A color is an int */
 typedef int color_t;
@@ -323,6 +327,9 @@ public:
     /** Whether a screen reset is needed after a repaint. */
     bool screen_reset_needed;
 
+    /** Whether the reader should exit on ^C. */
+    bool exit_on_interrupt;
+
     /** Constructor */
     reader_data_t() :
         allow_autosuggestion(0),
@@ -339,7 +346,8 @@ public:
         next(0),
         search_mode(0),
         repaint_needed(0),
-        screen_reset_needed(0)
+        screen_reset_needed(0),
+        exit_on_interrupt(0)
     {
     }
 };
@@ -373,7 +381,7 @@ static pid_t original_pid;
 /**
    This variable is set to true by the signal handler when ^C is pressed
 */
-static int interrupted=0;
+static volatile int interrupted=0;
 
 
 /*
@@ -579,6 +587,7 @@ static void reader_kill(size_t begin_idx, size_t length, int mode, int newv)
 
 }
 
+
 /* This is called from a signal handler! */
 void reader_handle_int(int sig)
 {
@@ -636,14 +645,42 @@ static void sort_and_make_unique(std::vector<completion_t> &l)
     l.erase(std::unique(l.begin(), l.end()), l.end());
 }
 
+
+void reader_reset_interrupted()
+{
+    interrupted = 0;
+}
+
 int reader_interrupted()
 {
-    int res=interrupted;
+    int res = interrupted;
     if (res)
+    {
         interrupted=0;
+    }
+    return res;
+}
+
+int reader_reading_interrupted()
+{
+    int res = reader_interrupted();
+    if (res && data && data->exit_on_interrupt)
+    {
+        reader_exit(1, 0);
+        parser_t::skip_all_blocks();
+        // We handled the interrupt ourselves, our caller doesn't need to 
+        // handle it.
+        return 0;
+    }
     return res;
 }
 
+bool reader_thread_job_is_stale()
+{
+    ASSERT_IS_BACKGROUND_THREAD();
+    return (void*)(uintptr_t) s_generation_count != pthread_getspecific(generation_count_key);
+}
+
 void reader_write_title()
 {
     const wchar_t *title;
@@ -768,6 +805,7 @@ static void exec_prompt()
 
 void reader_init()
 {
+    VOMIT_ON_FAILURE(pthread_key_create(&generation_count_key, NULL));
 
     tcgetattr(0,&shell_modes);        /* get the current terminal modes */
     memcpy(&saved_modes,
@@ -794,6 +832,7 @@ void reader_init()
 void reader_destroy()
 {
     tcsetattr(0, TCSANOW, &saved_modes);
+    pthread_key_delete(generation_count_key);
 }
 
 
@@ -1234,6 +1273,8 @@ struct autosuggestion_context_t
             return 0;
         }
 
+        VOMIT_ON_FAILURE(pthread_setspecific(generation_count_key, (void*)(uintptr_t) generation_count));
+
         /* Let's make sure we aren't using the empty string */
         if (search_string.empty())
         {
@@ -2416,6 +2457,11 @@ void reader_set_test_function(int (*f)(const wchar_t *))
     data->test_func = f;
 }
 
+void reader_set_exit_on_interrupt(bool i)
+{
+    data->exit_on_interrupt = i;
+}
+
 void reader_import_history_if_necessary(void)
 {
     /* Import history from bash, etc. if our current history is empty */
diff --git a/reader.h b/reader.h
index 1256c87b..ccd0dbf6 100644
--- a/reader.h
+++ b/reader.h
@@ -118,6 +118,29 @@ size_t reader_get_cursor_pos();
 int reader_interrupted();
 
 /**
+   Clear the interrupted flag unconditionally without handling anything. The 
+   flag could have been set e.g. when an interrupt arrived just as we were 
+   ending an earlier \c reader_readline invocation but before the 
+   \c is_interactive_read flag was cleared. 
+*/
+void reader_reset_interrupted();
+
+/**
+   Return the value of the interrupted flag, which is set by the sigint
+   handler, and clear it if it was set. If the current reader is interruptible,
+   call \c reader_exit(). 
+*/
+int reader_reading_interrupted();
+
+/**
+   Returns true if the current reader generation count does not equal the 
+   generation count the current thread was started with. 
+   Note 1: currently only valid for autocompletion threads! Other threads don't
+   set the threadlocal generation count when they start up. 
+*/
+bool reader_thread_job_is_stale();
+
+/**
    Read one line of input. Before calling this function, reader_push()
    must have been called in order to set up a valid reader
    environment.
@@ -181,6 +204,9 @@ void reader_set_right_prompt(const wcstring &prompt);
 /** Sets whether autosuggesting is allowed. */
 void reader_set_allow_autosuggesting(bool flag);
 
+/** Sets whether the reader should exit on ^C. */
+void reader_set_exit_on_interrupt(bool flag);
+
 /**
    Returns true if the shell is exiting, 0 otherwise.
 */
diff --git a/wildcard.cpp b/wildcard.cpp
index 43b37743..8f48848f 100644
--- a/wildcard.cpp
+++ b/wildcard.cpp
@@ -728,7 +728,7 @@ static int wildcard_expand_internal(const wchar_t *wc,
 
     //  debug( 3, L"WILDCARD_EXPAND %ls in %ls", wc, base_dir );
 
-    if (reader_interrupted())
+    if (is_main_thread() ? reader_interrupted() : reader_thread_job_is_stale())
     {
         return -1;
     }