Make commandline state thread safe

Today the reader exposes its internals directly, e.g. to the commandline
builtin. This is of course not thread safe. For example in concurrent
execution, running `commandline` twice in separate threads would cause a
race and likely a crash.

Fix this by factoring all the commandline state into a new type
'commandline_state_t'. Make it a singleton (there is only one command
line
after all) and protect it with a lock.

No user visible change here.

src/reader.cpp

@@ -123,6 +123,22 @@ enum class history_search_direction_t { forward, backward };
 enum class jump_direction_t { forward, backward };
 enum class jump_precision_t { till, to };
 
+/// A singleton snapshot of the reader state. This is updated when the reader changes. This is
+/// factored out for thread-safety reasons: it may be fetched on a background thread.
+static acquired_lock<commandline_state_t> commandline_state_snapshot() {
+    // Deliberately leaked to avoid shutdown dtors.
+    static owning_lock<commandline_state_t> *const s_state = new owning_lock<commandline_state_t>();
+    return s_state->acquire();
+}
+
+commandline_state_t commandline_get_state() { return *commandline_state_snapshot(); }
+
+void commandline_set_buffer(wcstring text, size_t cursor_pos) {
+    auto state = commandline_state_snapshot();
+    state->cursor_pos = std::min(cursor_pos, text.size());
+    state->text = std::move(text);
+}
+
 /// Any time the contents of a buffer changes, we update the generation count. This allows for our
 /// background threads to notice it and skip doing work that they would otherwise have to do.
 static std::atomic<uint32_t> s_generation;
@@ -715,6 +731,15 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
     /// \return the command, or none if we were asked to cancel (e.g. SIGHUP).
     maybe_t<wcstring> readline(int nchars);
 
+    /// Reflect our current data in the command line state snapshot.
+    /// This is called before we run any fish script, so that the commandline builtin can see our
+    /// state.
+    void update_commandline_state() const;
+
+    /// Apply any changes from the reader snapshot. This is called after running fish script,
+    /// incorporating changes from the commandline builtin.
+    void apply_commandline_state_changes();
+
     void move_word(editable_line_t *el, bool move_right, bool erase, enum move_word_style_t style,
                    bool newv);
 
@@ -725,6 +750,8 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
     void select_completion_in_direction(selection_motion_t dir);
     void flash();
 
+    maybe_t<source_range_t> get_selection() const;
+
     void completion_insert(const wcstring &val, size_t token_end, complete_flags_t flags);
 
     bool can_autosuggest() const;
@@ -1103,6 +1130,8 @@ void reader_data_t::command_line_changed(const editable_line_t *el) {
         this->pager.refilter_completions();
         this->pager_selection_changed();
     }
+    // Ensure that the commandline builtin sees our new state.
+    update_commandline_state();
 }
 
 void reader_data_t::pager_selection_changed() {
@@ -1849,6 +1878,14 @@ void reader_data_t::flash() {
     paint_layout(L"unflash");
 }
 
+maybe_t<source_range_t> reader_data_t::get_selection() const {
+    if (!this->selection.has_value()) return none();
+    size_t start = this->selection->start;
+    size_t len =
+        std::min(this->selection->stop, this->command_line.size()) - this->selection->start;
+    return source_range_t{static_cast<uint32_t>(start), static_cast<uint32_t>(len)};
+}
+
 /// Characters that may not be part of a token that is to be replaced by a case insensitive
 /// completion.
 #define REPLACE_UNCLEAN L"$*?({})"
@@ -2559,6 +2596,7 @@ void reader_change_history(const wcstring &name) {
     if (data && data->history) {
         data->history->save();
         data->history = history_t::with_name(name);
+        commandline_state_snapshot()->history = data->history;
     }
 }
 
@@ -2575,6 +2613,7 @@ static std::shared_ptr<reader_data_t> reader_push_ret(parser_t &parser,
     if (reader_data_stack.size() == 1) {
         reader_interactive_init(parser);
     }
+    data->update_commandline_state();
     return data;
 }
 
@@ -2589,8 +2628,10 @@ void reader_pop() {
     reader_data_t *new_reader = current_data_or_null();
     if (new_reader == nullptr) {
         reader_interactive_destroy();
+        *commandline_state_snapshot() = commandline_state_t{};
     } else {
         s_reset_abandoning_line(&new_reader->screen, termsize_last().width);
+        new_reader->update_commandline_state();
     }
 }
 
@@ -2661,6 +2702,29 @@ static bool selection_is_at_top(const reader_data_t *data) {
     return !(col != 0 && col != PAGER_SELECTION_NONE);
 }
 
+void reader_data_t::update_commandline_state() const {
+    auto snapshot = commandline_state_snapshot();
+    snapshot->text = this->command_line.text();
+    snapshot->cursor_pos = this->command_line.position();
+    snapshot->history = this->history;
+    snapshot->selection = this->get_selection();
+    snapshot->pager_mode = !this->current_page_rendering.screen_data.empty();
+    snapshot->search_mode = this->history_search.active();
+    snapshot->initialized = true;
+}
+
+void reader_data_t::apply_commandline_state_changes() {
+    // Only the text and cursor position may be changed.
+    commandline_state_t state = *commandline_state_snapshot();
+    if (state.text != this->command_line.text() ||
+        state.cursor_pos != this->command_line.position()) {
+        // The commandline builtin changed our contents.
+        this->pager.clear();
+        this->set_buffer_maintaining_pager(state.text, state.cursor_pos);
+        this->reset_loop_state = true;
+    }
+}
+
 static relaxed_atomic_t<uint64_t> run_count{0};
 
 /// Returns the current interactive loop count
@@ -2820,11 +2884,19 @@ struct readline_loop_state_t {
     size_t nchars{std::numeric_limits<size_t>::max()};
 };
 
+std::shared_ptr<history_t> reader_get_history() {
+    ASSERT_IS_MAIN_THREAD();
+    reader_data_t *data = current_data_or_null();
+    return data ? data->history : nullptr;
+}
+
 /// Run a sequence of commands from an input binding.
 void reader_data_t::run_input_command_scripts(const wcstring_list_t &cmds) {
     auto last_statuses = parser().get_last_statuses();
     for (const wcstring &cmd : cmds) {
+        update_commandline_state();
         parser().eval(cmd, io_chain_t{});
+        apply_commandline_state_changes();
     }
     parser().set_last_statuses(std::move(last_statuses));
 
@@ -3623,8 +3695,10 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
             editable_line_t *el = active_edit_line();
 
             // Check that we have an active selection and get the bounds.
-            size_t start, len;
-            if (reader_get_selection(&start, &len)) {
+            if (auto selection = this->get_selection()) {
+                size_t start = selection->start;
+                size_t len = selection->length;
+
                 size_t buff_pos = el->position();
                 wcstring replacement;
 
@@ -3714,9 +3788,8 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
 
         case rl::kill_selection: {
             bool newv = (rls.last_cmd != rl::kill_selection);
-            size_t start, len;
-            if (reader_get_selection(&start, &len)) {
-                kill(&command_line, start, len, KILL_APPEND, newv);
+            if (auto selection = this->get_selection()) {
+                kill(&command_line, selection->start, selection->length, KILL_APPEND, newv);
             }
             break;
         }
@@ -4089,15 +4162,6 @@ bool reader_data_t::jump(jump_direction_t dir, jump_precision_t precision, edita
 
 maybe_t<wcstring> reader_readline(int nchars) { return current_data()->readline(nchars); }
 
-bool reader_is_in_search_mode() {
-    reader_data_t *data = current_data_or_null();
-    return data && data->history_search.active();
-}
-
-bool reader_has_pager_contents() {
-    reader_data_t *data = current_data_or_null();
-    return data && !data->current_page_rendering.screen_data.empty();
-}
 
 int reader_reading_interrupted() {
     int res = reader_test_and_clear_interrupted();
@@ -4132,18 +4196,6 @@ void reader_queue_ch(const char_event_t &ch) {
     }
 }
 
-const wchar_t *reader_get_buffer() {
-    ASSERT_IS_MAIN_THREAD();
-    reader_data_t *data = current_data_or_null();
-    return data ? data->command_line.text().c_str() : nullptr;
-}
-
-std::shared_ptr<history_t> reader_get_history() {
-    ASSERT_IS_MAIN_THREAD();
-    reader_data_t *data = current_data_or_null();
-    return data ? data->history : nullptr;
-}
-
 /// Sets the command line contents, clearing the pager.
 void reader_set_buffer(const wcstring &b, size_t pos) {
     reader_data_t *data = current_data_or_null();
@@ -4154,23 +4206,6 @@ void reader_set_buffer(const wcstring &b, size_t pos) {
     data->reset_loop_state = true;
 }
 
-size_t reader_get_cursor_pos() {
-    reader_data_t *data = current_data_or_null();
-    if (!data) return static_cast<size_t>(-1);
-
-    return data->command_line.position();
-}
-
-bool reader_get_selection(size_t *start, size_t *len) {
-    bool result = false;
-    reader_data_t *data = current_data_or_null();
-    if (data != nullptr && data->selection.has_value()) {
-        *start = data->selection->start;
-        *len = std::min(data->selection->stop, data->command_line.size()) - data->selection->start;
-        result = true;
-    }
-    return result;
-}
 
 /// Read non-interactively.  Read input from stdin without displaying the prompt, using syntax
 /// highlighting. This is used for reading scripts and init files.