From 268fb375171668e2d85a147c7614e89ce1bd46f6 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sat, 31 Mar 2012 15:33:34 -0700
Subject: [PATCH] Cleanup proc_had_barrier Ensure we don't try to do a
 universal barrier off of the main thread

---
 env.cpp           | 88 +++++++++++++++++++++++++----------------------
 env_universal.cpp |  1 +
 parser.cpp        |  3 +-
 proc.cpp          | 13 ++++++-
 proc.h            |  3 +-
 5 files changed, 63 insertions(+), 45 deletions(-)

diff --git a/env.cpp b/env.cpp
index 034a72f01..a7a263986 100644
--- a/env.cpp
+++ b/env.cpp
@@ -730,6 +730,7 @@ static env_node_t *env_get_node( const wcstring &key )
 
 int env_set(const wchar_t *key,  const wchar_t *val, int var_mode)
 {
+    ASSERT_IS_MAIN_THREAD();
 	env_node_t *node = NULL;
 	bool has_changed_old = has_changed;
 	bool has_changed_new = false;
@@ -840,9 +841,9 @@ int env_set(const wchar_t *key,  const wchar_t *val, int var_mode)
 			}
 			else
 			{
-				if( !proc_had_barrier)
+				if( ! get_proc_had_barrier())
 				{
-					proc_had_barrier=1;
+					set_proc_had_barrier(true);
 					env_universal_barrier();
 				}
 				
@@ -1062,7 +1063,8 @@ const wchar_t *env_var_t::c_str(void) const {
 env_var_t env_get_string( const wcstring &key )
 {    
     /* Big hack...we only allow getting the history on the main thread. Note that history_t may ask for an environment variable, so don't take the lock here (we don't need it) */
-	if( key == L"history" && is_main_thread())
+    const bool is_main = is_main_thread();
+	if( key == L"history" && is_main)
 	{
         env_var_t result;
         
@@ -1100,42 +1102,44 @@ env_var_t env_get_string( const wcstring &key )
         wcstring result;
         
         while( env != 0 )
-	  {
-		var_table_t::iterator result =  env->env.find(key);
-	    if ( result != env->env.end() ) 
-	      {
-			res = result->second; 
-	      }
-	    else
-	      {
-			res = 0;
-	      }
-	
-
-	    if( res != 0 )
-	      {
-		if( res->val == ENV_NULL ) 
-		  {
-		    return env_var_t::missing_var();
-		  }
-		else
-		  {
-		    return res->val;			
-		  }
-	      }
-            
-	    if( env->new_scope )
-	      {
-                env = global_env;
-	      }
-	    else
-	      {
-                env = env->next;
-	      }
-	  }	
-        if( !proc_had_barrier)
         {
-            proc_had_barrier=1;
+            var_table_t::iterator result =  env->env.find(key);
+            if ( result != env->env.end() ) 
+            {
+                res = result->second; 
+            }
+            else
+            {
+                res = 0;
+            }
+            
+            
+            if( res != 0 )
+            {
+                if( res->val == ENV_NULL ) 
+                {
+                    return env_var_t::missing_var();
+                }
+                else
+                {
+                    return res->val;			
+                }
+            }
+            
+            if( env->new_scope )
+            {
+                env = global_env;
+            }
+            else
+            {
+                env = env->next;
+            }
+        }
+        
+        /* Another big hack - only do a universal barrier on the main thread (since it can change variable values) */
+        if(is_main && ! get_proc_had_barrier())
+        {
+            set_proc_had_barrier(true);
             env_universal_barrier();
         }
         
@@ -1211,9 +1215,9 @@ int env_exist( const wchar_t *key, int mode )
 	
 	if( ! (mode & ENV_LOCAL) && ! (mode & ENV_GLOBAL) )
 	{
-		if( !proc_had_barrier)
+		if( ! get_proc_had_barrier())
 		{
-			proc_had_barrier=1;
+			set_proc_had_barrier(true);
 			env_universal_barrier();
 		}
 		
@@ -1453,9 +1457,9 @@ static void export_func(const std::map<wcstring, wcstring> &envs, std::vector<st
 
 static void update_export_array_if_necessary(bool recalc) {
     ASSERT_IS_MAIN_THREAD();
-	if( recalc && !proc_had_barrier)
+	if( recalc && ! get_proc_had_barrier())
 	{
-		proc_had_barrier=1;
+		set_proc_had_barrier(true);
 		env_universal_barrier();
 	}
 	
diff --git a/env_universal.cpp b/env_universal.cpp
index a7d6dcae0..e9b0d0d86 100644
--- a/env_universal.cpp
+++ b/env_universal.cpp
@@ -338,6 +338,7 @@ int env_universal_get_export( const wchar_t *name )
 
 void env_universal_barrier()
 {
+    ASSERT_IS_MAIN_THREAD();
 	message_t *msg;
 	fd_set fds;
 
diff --git a/parser.cpp b/parser.cpp
index 64c323db7..6c6f2f793 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -2251,6 +2251,7 @@ void parser_t::skipped_exec( job_t * j )
 
 void parser_t::eval_job( tokenizer *tok )
 {
+    ASSERT_IS_MAIN_THREAD();
 	job_t *j;
 
 	int start_pos = job_start_pos = tok_get_pos( tok );
@@ -2340,7 +2341,7 @@ void parser_t::eval_job( tokenizer *tok )
 					
 					/* Only external commands require a new fishd barrier */
 					if( !was_builtin )
-						proc_had_barrier=0;
+						set_proc_had_barrier(false);
 				}
 				else
 				{
diff --git a/proc.cpp b/proc.cpp
index 2bf212db0..e6cc4ab2d 100644
--- a/proc.cpp
+++ b/proc.cpp
@@ -122,18 +122,29 @@ int is_subshell=0;
 int is_block=0;
 int is_login=0;
 int is_event=0;
-int proc_had_barrier;
 pid_t proc_last_bg_pid = 0;
 int job_control_mode = JOB_CONTROL_INTERACTIVE;
 int no_exec=0;
 
 static int is_interactive = -1;
 
+static bool proc_had_barrier = false;
+
 int get_is_interactive(void) {
     ASSERT_IS_MAIN_THREAD();
     return is_interactive;
 }
 
+bool get_proc_had_barrier() {
+    ASSERT_IS_MAIN_THREAD();
+    return proc_had_barrier;
+}
+
+void set_proc_had_barrier(bool flag) {
+    ASSERT_IS_MAIN_THREAD();
+    proc_had_barrier = flag;
+}
+
 /**
    The event variable used to send all process event
 */
diff --git a/proc.h b/proc.h
index 45de45a18..ae8a85434 100644
--- a/proc.h
+++ b/proc.h
@@ -464,7 +464,8 @@ class job_iterator_t {
    Both setting it to one when it should be zero and the opposite may
    cause concurrency bugs.
 */
-extern int proc_had_barrier;
+bool get_proc_had_barrier();
+void set_proc_had_barrier(bool flag);
 
 /**
    Pid of last process to be started in the background