Update autoloader to fix concurrency issues when changing loader path in autoloaded function

darcs-hash:20061028164122-ac50b-25f978df9afeb370a06ef7576ef03183034bc057.gz

function.c

@@ -353,6 +353,9 @@ const wchar_t *function_get_definition_file( const wchar_t *argv )
 
 	CHECK( argv, 0 );
 		
+	if( is_autoload )
+		return 0;
+
 	load( argv );
 	data = (function_data_t *)hash_get( &function, argv );
 	if( data == 0 )
@@ -368,6 +371,9 @@ int function_get_definition_offset( const wchar_t *argv )
 
 	CHECK( argv, -1 );
 		
+	if( is_autoload )
+		return -1;
+
 	load( argv );
 	data = (function_data_t *)hash_get( &function, argv );
 	if( data == 0 )

parse_util.c

@@ -2,6 +2,10 @@
 
     Various mostly unrelated utility functions related to parsing,
     loading and evaluating fish code.
+
+	This library can be seen as a 'toolbox' for functions that are
+	used in many places in fish and that are somehow related to
+	parsing the code. 
 */
 
 #include "config.h"
@@ -491,9 +495,18 @@ static void clear_hash_value( void *key, void *data, void *aux )
 	if( aux )
 	{
 		wchar_t *name = (wchar_t *)key;
+		time_t *time = (time_t *)data;
 		void (*handler)(const wchar_t *)= (void (*)(const wchar_t *))aux;
-		if( handler )
+
+		/*
+		  If time[0] is 0, that means the file really doesn't exist,
+		  it's simply been checked before. We should not unload it.
+		*/
+		if( time[0] && handler )
+		{
+//			debug( 0, L"Unloading function %ls", name );
 			handler( name );
+		}
 	}
 	
 	free( (void *)data );
@@ -540,14 +553,8 @@ static void parse_util_destroy()
 void parse_util_load_reset( const wchar_t *path_var_name,
 							void (*on_load)(const wchar_t *cmd) )
 {
-	wchar_t *path_var;
-	
 	CHECK( path_var_name, );
-	path_var = env_get( path_var_name );	
 
-	if( !path_var )
-		return;
-		
 	if( all_loaded )
 	{
 		void *key, *data;
@@ -654,7 +661,6 @@ static void parse_util_autounload( const wchar_t *path_var_name,
 	}
 }
 
-
 static int parse_util_load_internal( const wchar_t *cmd,
 									 void (*on_load)(const wchar_t *cmd),
 									 int reload,
@@ -679,10 +685,10 @@ int parse_util_load( const wchar_t *cmd,
 	CHECK( path_var_name, 0 );
 	CHECK( cmd, 0 );
 
-	parse_util_autounload( path_var_name, cmd, on_load );
-	
 //	debug( 0, L"Autoload %ls in %ls", cmd, path_var_name );
 
+	parse_util_autounload( path_var_name, cmd, on_load );
+	
 	path_var = env_get( path_var_name );	
 	
 	/*
@@ -712,6 +718,18 @@ int parse_util_load( const wchar_t *cmd,
 
 	if( loaded )
 	{
+		/*
+		  Check if the lookup path has changed. If so, drop all loaded
+		  files and start from scratch.
+		*/
+		if( wcscmp( path_var, loaded->old_path ) != 0 )
+		{
+			debug( 0, L"path change, new path is %ls", path_var );			
+			parse_util_load_reset( path_var_name, on_load);
+			reload = parse_util_load( cmd, path_var_name, on_load, reload );
+			return reload;
+		}
+
 		/**
 		   Warn and fail on infinite recursion
 		*/
@@ -724,17 +742,7 @@ int parse_util_load( const wchar_t *cmd,
 			return 1;
 		}
 		
-		/*
+
-		  Check if the lookup path has changed. If so, drop all loaded
-		  files and start from scratch.
-		*/
-		if( wcscmp( path_var, loaded->old_path ) != 0 )
-		{
-			parse_util_load_reset( path_var_name, on_load);
-			reload = parse_util_load( cmd, path_var_name, on_load, reload );
-//			debug( 0, L"Reload" );
-			return reload;
-		}
 	}
 	else
 	{
@@ -742,6 +750,7 @@ int parse_util_load( const wchar_t *cmd,
 		  We have never tried to autoload using this path name before,
 		  set up initial data
 		*/
+//		debug( 0, L"Create brand new autoload_t for %ls->%ls", path_var_name, path_var );
 		loaded = malloc( sizeof( autoload_t ) );
 		if( !loaded )
 		{
@@ -819,7 +828,6 @@ static int parse_util_load_internal( const wchar_t *cmd,
 	{
 		if(time(0)-tm[1]<=1)
 		{
-//			debug( 0, L"Cached" );
 			return 0;
 		}
 	}
@@ -829,8 +837,6 @@ static int parse_util_load_internal( const wchar_t *cmd,
 	*/
 	if( !reload && tm )
 	{
-//		debug( 0, L"Weak check" );
-
 		return 0;
 	}
 	
@@ -871,10 +877,10 @@ static int parse_util_load_internal( const wchar_t *cmd,
 						  intern( cmd ),
 						  tm );
 
-
 				if( on_load )
 					on_load(cmd );
 
+
 				/*
 				  Source the completion file for the specified completion
 				*/
@@ -888,6 +894,15 @@ static int parse_util_load_internal( const wchar_t *cmd,
 				free(src_cmd);
 				reloaded = 1;
 			}
+			else if( tm )
+			{
+				/*
+				  If we are rechecking an autoload file, and it hasn't
+				  changed, update the 'last check' timestamp.
+				*/
+				tm[1] = time(0);				
+			}
+			
 			break;
 		}
 	}
@@ -908,8 +923,6 @@ static int parse_util_load_internal( const wchar_t *cmd,
 		hash_put( &loaded->load_time, intern( cmd ), tm );
 	}
 
-//	debug( 0, L"Regular return" );
-
 	return reloaded;	
 }
 

parser.c

@@ -470,6 +470,17 @@ void parser_push_block( int type )
 void parser_pop_block()
 {
 	block_t *old = current_block;
+	if( !current_block )
+	{
+		debug( 1,
+			   L"function %s called on empty block stack. "
+			   L"This is a bug. "
+			   L"If you can reproduce it, please send a bug report to %s.",
+			   __func__,
+			   PACKAGE_BUGREPORT );										\
+		return;
+	}
+	
 	current_block = current_block->outer;
 	halloc_free( old );
 }
@@ -495,7 +506,6 @@ const wchar_t *parser_get_block_desc( int block )
 */
 static int parser_skip_arguments( const wchar_t *cmd )
 {
-
 	return contains_str( cmd,
 						 L"else",
 						 L"begin",
@@ -569,6 +579,8 @@ static const wchar_t *parser_find_end( const wchar_t * buff )
 	int error=0;
 	int mark=0;
 
+	CHECK( buff, 0 );
+
 	for( tok_init( &tok, buff, 0 );
 		 tok_has_next( &tok ) && !error;
 		 tok_next( &tok ) )
@@ -652,6 +664,7 @@ void parser_forbid_function( wchar_t *function )
   if( function )
   debug( 2, L"Forbid %ls\n", function );
 */
+	CHECK( function, );
 	al_push( forbidden_function, function?wcsdup(function):0 );
 }
 
@@ -668,6 +681,8 @@ void error( int ec, int p, const wchar_t *str, ... )
 {
 	va_list va;
 
+	CHECK( str, );
+	
 	if( !err_buff )
 		err_buff = sb_halloc( global_context );
 	sb_clear( err_buff );	
@@ -820,6 +835,9 @@ void parser_destroy()
 */
 static void print_errors( string_buffer_t *target, const wchar_t *prefix )
 {
+	CHECK( target, );
+	CHECK( prefix, );
+	
 	if( error_code && err_buff )
 	{
 		int tmp;
@@ -1042,9 +1060,10 @@ void parser_stack_trace( block_t *b, string_buffer_t *buff)
 }
 
 /**
-   Returns true if we are currently evaluating a function. This is
+   Returns the name of the currently evaluated function if we are
-   tested by moving down the block-scope-stack, checking every block
+   currently evaluating a function, null otherwise. This is tested by
-   if it is of type FUNCTION_CALL.
+   moving down the block-scope-stack, checking every block if it is of
+   type FUNCTION_CALL.
 */
 static const wchar_t *is_function()
 {
@@ -1121,6 +1140,9 @@ static int printed_width( const wchar_t *str, int len )
 {
 	int res=0;
 	int i;
+
+	CHECK( str, 0 );
+
 	for( i=0; str[i] && i<len; i++ )
 	{
 		if( str[i] == L'\t' )
@@ -1279,7 +1301,11 @@ const wchar_t *parser_get_buffer()
 
 int parser_is_help( wchar_t *s, int min_match )
 {
-	int len = wcslen(s);
+	int len;
+
+	CHECK( s, 0 );
+
+	len = wcslen(s);
 
 	min_match = maxi( min_match, 3 );
 		
@@ -1901,18 +1927,21 @@ static int parse_job( process_t *p,
 		
 		if( use_function && !current_block->skip )
 		{
-			int nxt_forbidden;
+			int nxt_forbidden=0;
 			wchar_t *forbid;
 
-			forbid = (wchar_t *)(al_get_count( forbidden_function)?al_peek( forbidden_function ):0);
+			if( current_block->type == FUNCTION_CALL )
-			nxt_forbidden = forbid && (wcscmp( forbid, nxt) == 0 );
+			{
+				forbid = (wchar_t *)(al_get_count( forbidden_function)?al_peek( forbidden_function ):0);
+				nxt_forbidden = forbid && (wcscmp( forbid, nxt) == 0 );
+			}
 			
 			/*
 			  Make feeble attempt to avoid infinite recursion. Will at
 			  least catch some accidental infinite recursion calls.
 			*/
-			
+		
-			if( function_exists( nxt ) && !nxt_forbidden)
+			if( !nxt_forbidden && function_exists( nxt ) )
 			{
 				/*
 				  Check if we have reached the maximum recursion depth