From 7f59a7e7cfd2d49d8df3d652ff1e1b8a0b1527bc Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Thu, 7 Nov 2019 23:15:26 -0800
Subject: [PATCH] Only dispatch variable changes for the principal variable
 stack or globals

fish will react to certain variable modifications, such as "TZ." Only do
this if the main stack is modified. This has no effect now because there
is always a single stack, but will become important when concurrent
execution is supported.
---
 src/env.cpp | 26 ++++++++++++++++++--------
 src/env.h   |  3 +++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/env.cpp b/src/env.cpp
index 1b1ce107c..2eac7d73f 100644
--- a/src/env.cpp
+++ b/src/env.cpp
@@ -1264,13 +1264,17 @@ int env_stack_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t
 
     mod_result_t ret = acquire_impl()->set(key, mode, std::move(vals));
     if (ret.status == ENV_OK) {
+        // If we modified the global state, or we are principal, then dispatch changes.
         // Important to not hold the lock here.
-        env_dispatch_var_change(key, *this);
+        if (ret.global_modified || is_principal()) {
+            env_dispatch_var_change(key, *this);
+        }
         if (out_events) {
             out_events->push_back(event_t::variable(key, {L"VARIABLE", L"SET", key}));
         }
     }
-    if (ret.uvar_modified) {
+    // If the principal stack modified universal variables, then post a barrier.
+    if (ret.uvar_modified && is_principal()) {
         universal_barrier();
     }
     return ret.status;
@@ -1291,13 +1295,15 @@ int env_stack_t::set_empty(const wcstring &key, env_mode_flags_t mode,
 int env_stack_t::remove(const wcstring &key, int mode, std::vector<event_t> *out_events) {
     mod_result_t ret = acquire_impl()->remove(key, mode);
     if (ret.status == ENV_OK) {
-        // Important to not hold the lock here.
-        env_dispatch_var_change(key, *this);
+        if (ret.global_modified || is_principal()) {
+            // Important to not hold the lock here.
+            env_dispatch_var_change(key, *this);
+        }
         if (out_events) {
             out_events->push_back(event_t::variable(key, {L"VARIABLE", L"ERASE", key}));
         }
     }
-    if (ret.uvar_modified) {
+    if (ret.uvar_modified && is_principal()) {
         universal_barrier();
     }
     return ret.status;
@@ -1330,9 +1336,13 @@ void env_stack_t::push(bool new_scope) {
 
 void env_stack_t::pop() {
     auto popped = acquire_impl()->pop();
-    // TODO: we would like to coalesce locale / curses changes, so that we only re-initialize once.
-    for (const auto &kv : popped->env) {
-        env_dispatch_var_change(kv.first, *this);
+    // Only dispatch variable changes if we are the principal environment.
+    if (this == principal_ref().get()) {
+        // TODO: we would like to coalesce locale / curses changes, so that we only re-initialize
+        // once.
+        for (const auto &kv : popped->env) {
+            env_dispatch_var_change(kv.first, *this);
+        }
     }
 }
 
diff --git a/src/env.h b/src/env.h
index a14b0dcdb..454cfd94f 100644
--- a/src/env.h
+++ b/src/env.h
@@ -226,6 +226,9 @@ class env_stack_t final : public environment_t {
 
     explicit env_stack_t(std::unique_ptr<env_stack_impl_t> impl);
 
+    /// \return whether we are the principal stack.
+    bool is_principal() const { return this == principal_ref().get(); }
+
    public:
     ~env_stack_t() override;
     env_stack_t(env_stack_t &&);