Restructure the autoloader. This fixes a very weird and rare Heissenbug, probably caused by a compiler bug, but also makes the structure of the code a bit more logical.

darcs-hash:20060828110019-ac50b-7be19eda18eb358163a9a5b7180c6c7d028e546d.gz

parse_util.c

@@ -580,34 +580,46 @@ int parse_util_unload( const wchar_t *cmd,
 	return !!val;
 }
 
+static int path_util_load_internal( const wchar_t *cmd,
+									void (*on_load)(const wchar_t *cmd),
+									int reload,
+									autoload_t *loaded,
+									array_list_t *path_list );
+
+
 int parse_util_load( const wchar_t *cmd,
 					 const wchar_t *path_var_name,
 					 void (*on_load)(const wchar_t *cmd),
 					 int reload )
 {
 	static array_list_t *path_list=0;
-	static string_buffer_t *path=0;
 
-	int i;
-	time_t *tm;
-	int reloaded = 0;
 	autoload_t *loaded;
 
 	wchar_t *path_var;
 
+	int res;
+	int c, c2;
+	
 	CHECK( path_var_name, 0 );
 	CHECK( cmd, 0 );
 	
+//	debug( 0, L"Autoload %ls in %ls", cmd, path_var_name );
+
 	path_var = env_get( path_var_name );	
 	
 	/*
-	  Do we know where to look
+	  Do we know where to look?
 	*/
 	if( !path_var )
 	{
+//		debug( 0, L"Path null" );
 		return 0;
 	}
-	
+
+	/**
+	   Init if this is the first time we try to autoload anything
+	*/
 	if( !all_loaded )
 	{
 		all_loaded = malloc( sizeof( hash_table_t ) );		
@@ -623,9 +635,14 @@ int parse_util_load( const wchar_t *cmd,
 
 	if( loaded )
 	{
+		/**
+		   Warn and fail on infinite recursion
+		*/
 		if( hash_get( &loaded->is_loading, cmd ) )
 		{
-			debug( 0, _(L"Could not autoload item %ls, it is already being autoloaded. This is a circular dependency in the autoloading scripts, please remove it."), cmd );
+			debug( 0, 
+				   _(L"Could not autoload item '%ls', it is already being autoloaded. This is a circular dependency in the autoloading scripts, please remove it."), 
+				   cmd );
 			return 1;
 		}
 		
@@ -637,35 +654,87 @@ int parse_util_load( const wchar_t *cmd,
 		{
 			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
 	{
 		/*
-		  We have never tried to autoload using this name before, set up initial data
+		  We have never tried to autoload using this path name before,
+		  set up initial data
 		*/
- 		loaded = malloc( sizeof( autoload_t ) );
+		loaded = malloc( sizeof( autoload_t ) );
 		if( !loaded )
 		{
 			DIE_MEM();
 		}
 		hash_init( &loaded->load_time, &hash_wcs_func, &hash_wcs_cmp );
 		hash_put( all_loaded, wcsdup(path_var_name), loaded );
-
+		
 		hash_init( &loaded->is_loading, &hash_wcs_func, &hash_wcs_cmp );
 
 		loaded->old_path = wcsdup( path_var );
 	}
 
-	hash_put( &loaded->is_loading, cmd, cmd );
-	
 
+	if( !path_list )
+		path_list = al_halloc( global_context);
+	
+	al_truncate( path_list, 0 );
+	
+	tokenize_variable_array( path_var, path_list );
+	
+	c = hash_get_count( &loaded->is_loading );
+	
+	hash_put( &loaded->is_loading, cmd, cmd );
+
+	/*
+	  Do the actual work in the internal helper function
+	*/
+
+	res = path_util_load_internal( cmd, on_load, reload, loaded, path_list );
+
+	/**
+	   Cleanup
+	*/
+	hash_remove( &loaded->is_loading, cmd, 0, 0 );
+
+	al_foreach( path_list, &free );
+	al_truncate( path_list, 0 );
+	
+	/**
+	   Make sure we didn't 'drop' something
+	*/
+
+	c2 = hash_get_count( &loaded->is_loading );
+	assert( c == c2 );
+	
+	return res;
+}
+
+/**
+   This internal helper function does all the real work. By using two
+   functions, the internal function can return on various places in
+   the code, and the caller can take care of various cleanup work.
+*/
+
+static int path_util_load_internal( const wchar_t *cmd,
+									void (*on_load)(const wchar_t *cmd),
+									int reload,
+									autoload_t *loaded,
+									array_list_t *path_list )
+{
+	static string_buffer_t *path=0;
+	time_t *tm;
+	int i;
+	int reloaded = 0;
+		
 	/*
 	  Get modification time of file
 	*/
 	tm = (time_t *)hash_get( &loaded->load_time, cmd );
-
+	
 	/*
 	  Did we just check this?
 	*/
@@ -673,7 +742,7 @@ int parse_util_load( const wchar_t *cmd,
 	{
 		if(time(0)-tm[1]<=1)
 		{
-			hash_remove( &loaded->is_loading, cmd, 0, 0 );
+//			debug( 0, L"Cached" );
 			return 0;
 		}
 	}
@@ -683,20 +752,16 @@ int parse_util_load( const wchar_t *cmd,
 	*/
 	if( !reload && tm )
 	{
-		hash_remove( &loaded->is_loading, cmd, 0, 0 );
+//		debug( 0, L"Weak check" );
+
 		return 0;
 	}
 	
-	if( !path_list )
-		path_list = al_halloc( global_context);
-	
 	if( !path )
 		path = sb_halloc( global_context );
 	else
 		sb_clear( path );
 	
-	tokenize_variable_array( path_var, path_list );
-	
 	/*
 	  Iterate over path searching for suitable completion files
 	*/
@@ -759,10 +824,8 @@ int parse_util_load( const wchar_t *cmd,
 		hash_put( &loaded->load_time, intern( cmd ), tm );
 	}
 
-	al_foreach( path_list, &free );
+//	debug( 0, L"Regular return" );
-	al_truncate( path_list, 0 );
+
-	
-	hash_remove( &loaded->is_loading, cmd, 0, 0 );
 	return reloaded;	
 }