Some initial changes to use CLO_EXEC, with an eye towards some day using it correctly.

builtin.cpp

@@ -2724,6 +2724,7 @@ static int builtin_contains( parser_t &parser, wchar_t ** argv )
 */
 static int builtin_source( parser_t &parser, wchar_t ** argv )
 {
+    ASSERT_IS_MAIN_THREAD();
 	int fd;
 	int res = STATUS_BUILTIN_OK;
 	struct stat buf;
@@ -2759,7 +2760,7 @@ static int builtin_source( parser_t &parser, wchar_t ** argv )
 			return STATUS_BUILTIN_ERROR;
 		}
 
-		if( ( fd = wopen( argv[1], O_RDONLY ) ) == -1 )
+		if( ( fd = wopen_cloexec( argv[1], O_RDONLY ) ) == -1 )
 		{
 			append_format(stderr_buffer, _(L"%ls: Error encountered while sourcing file '%ls':\n"), argv[0], argv[1] );
 			builtin_wperror( L"." );

common.cpp

@@ -49,14 +49,6 @@ parts of fish.
 #include <execinfo.h>
 #endif
 
-#ifndef HOST_NAME_MAX
-/**
-   Maximum length of hostname return. It is ok if this is too short,
-   getting the actual hostname is not critical, so long as the string
-   is unique in the filesystem namespace.
-*/
-#define HOST_NAME_MAX 255
-#endif
 
 #if HAVE_NCURSES_H
 #include <ncurses.h>
@@ -88,11 +80,7 @@ parts of fish.
 #include "util.cpp"
 #include "fallback.cpp"
 
-/**
+
-   The number of milliseconds to wait between polls when attempting to acquire 
-   a lockfile
-*/
-#define LOCKPOLLINTERVAL 10
 
 struct termios shell_modes;      
 
@@ -1693,231 +1681,7 @@ bool unescape_string(wcstring &str, int escape_special)
     return success;
 }
 
-/**
-   Writes a pid_t in decimal representation to str.
-   str must contain sufficient space.
-   The conservatively approximate maximum number of characters a pid_t will 
-   represent is given by: (int)(0.31 * sizeof(pid_t) + CHAR_BIT + 1)
-   Returns the length of the string
-*/
-static int sprint_pid_t( pid_t pid, char *str )
-{
-	int len, i = 0;
-	int dig;
 
-	/* Store digits in reverse order into string */
-	while( pid != 0 )
-	{
-		dig = pid % 10;
-		str[i] = '0' + dig;
-		pid = ( pid - dig ) / 10;
-		i++;
-	}
-	len = i;
-	/* Reverse digits */
-	i /= 2;
-	while( i )
-	{
-		i--;
-		dig = str[i];
-		str[i] = str[len -  1 - i];
-		str[len - 1 - i] = dig;
-	}
-	return len;
-}
-
-/**
-   Writes a pseudo-random number (between one and maxlen) of pseudo-random
-   digits into str.
-   str must point to an allocated buffer of size of at least maxlen chars.
-   Returns the number of digits written.
-   Since the randomness in part depends on machine time it has _some_ extra
-   strength but still not enough for use in concurrent locking schemes on a
-   single machine because gettimeofday may not return a different value on
-   consecutive calls when:
-   a) the OS does not support fine enough resolution
-   b) the OS is running on an SMP machine.
-   Additionally, gettimeofday errors are ignored.
-   Excludes chars other than digits since ANSI C only guarantees that digits
-   are consecutive.
-*/
-static int sprint_rand_digits( char *str, int maxlen )
-{
-	int i, max;
-	struct timeval tv;
-	
-	/* 
-	   Seed the pseudo-random generator based on time - this assumes
-	   that consecutive calls to gettimeofday will return different values
-	   and ignores errors returned by gettimeofday.
-	   Cast to unsigned so that wrapping occurs on overflow as per ANSI C.
-	*/
-	(void)gettimeofday( &tv, NULL );
-	srand( (unsigned int)tv.tv_sec + (unsigned int)tv.tv_usec * 1000000UL );
-	max = 1 + (maxlen - 1) * (rand() / (RAND_MAX + 1.0));
-	for( i = 0; i < max; i++ )
-	{
-		str[i] = '0' + 10 * (rand() / (RAND_MAX + 1.0));
-	}
-	return i;
-}
-
-/**
-   Generate a filename unique in an NFS namespace by creating a copy of str and
-   appending .{hostname}.{pid} to it.  If gethostname() fails then a pseudo-
-   random string is substituted for {hostname} - the randomness of the string
-   should be strong enough across different machines.  The main assumption 
-   though is that gethostname will not fail and this is just a "safe enough"
-   fallback.
-   The memory returned should be freed using free().
-*/
-static char *gen_unique_nfs_filename( const char *filename )
-{
-	int pidlen, hnlen, orglen = strlen( filename );
-	char hostname[HOST_NAME_MAX + 1];
-	char *newname;
-	
-	if ( gethostname( hostname, HOST_NAME_MAX + 1 ) == 0 )
-	{
-		hnlen = strlen( hostname );
-	}
-	else
-	{
-		hnlen = sprint_rand_digits( hostname, HOST_NAME_MAX );
-		hostname[hnlen] = '\0';
-	}
-	newname = (char *)malloc( orglen + 1 /* period */ + hnlen + 1 /* period */ +
-					  /* max possible pid size: 0.31 ~= log(10)2 */
-					  (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1)
-					  + 1 /* '\0' */ );
-	
-	if ( newname == NULL )
-	{
-		debug( 1, L"gen_unique_nfs_filename: %s", strerror( errno ) );
-		return newname;
-	}
-	memcpy( newname, filename, orglen );
-	newname[orglen] = '.';
-	memcpy( newname + orglen + 1, hostname, hnlen );
-	newname[orglen + 1 + hnlen] = '.';
-	pidlen = sprint_pid_t( getpid(), newname + orglen + 1 + hnlen + 1 );
-	newname[orglen + 1 + hnlen + 1 + pidlen] = '\0';
-/*	debug( 1, L"gen_unique_nfs_filename returning with: newname = \"%s\"; "
-  L"HOST_NAME_MAX = %d; hnlen = %d; orglen = %d; "
-  L"sizeof(pid_t) = %d; maxpiddigits = %d; malloc'd size: %d", 
-  newname, (int)HOST_NAME_MAX, hnlen, orglen, 
-  (int)sizeof(pid_t), 
-  (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1),
-  (int)(orglen + 1 + hnlen + 1 + 
-  (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1) + 1) ); */
-	return newname;
-}
-
-int acquire_lock_file( const char *lockfile, const int timeout, int force )
-{
-	int fd, timed_out = 0;
-	int ret = 0; /* early exit returns failure */
-	struct timespec pollint;
-	struct timeval start, end;
-	double elapsed;
-	struct stat statbuf;
-
-	/*
-	  (Re)create a unique file and check that it has one only link.
-	*/
-	char *linkfile = gen_unique_nfs_filename( lockfile );
-	if( linkfile == NULL )
-	{
-		goto done;
-	}
-	(void)unlink( linkfile );
-	if( ( fd = open( linkfile, O_CREAT|O_RDONLY, 0600 ) ) == -1 )
-	{
-		debug( 1, L"acquire_lock_file: open: %s", strerror( errno ) );
-		goto done;
-	}
-	/*
-	  Don't need to check exit status of close on read-only file descriptors
-	*/
-	close( fd );
-	if( stat( linkfile, &statbuf ) != 0 )
-	{
-		debug( 1, L"acquire_lock_file: stat: %s", strerror( errno ) );
-		goto done;
-	}
-	if ( statbuf.st_nlink != 1 )
-	{
-		debug( 1, L"acquire_lock_file: number of hardlinks on unique "
-			   L"tmpfile is %d instead of 1.", (int)statbuf.st_nlink );
-		goto done;
-	}
-	if( gettimeofday( &start, NULL ) != 0 )
-	{
-		debug( 1, L"acquire_lock_file: gettimeofday: %s", strerror( errno ) );
-		goto done;
-	}
-	end = start;
-	pollint.tv_sec = 0;
-	pollint.tv_nsec = LOCKPOLLINTERVAL * 1000000;
-	do 
-	{
-		/*
-		  Try to create a hard link to the unique file from the 
-		  lockfile.  This will only succeed if the lockfile does not 
-		  already exist.  It is guaranteed to provide race-free 
-		  semantics over NFS which the alternative of calling 
-		  open(O_EXCL|O_CREAT) on the lockfile is not.  The lock 
-		  succeeds if the call to link returns 0 or the link count on 
-		  the unique file increases to 2.
-		*/
-		if( link( linkfile, lockfile ) == 0 ||
-		    ( stat( linkfile, &statbuf ) == 0 && 
-			  statbuf.st_nlink == 2 ) )
-		{
-			/* Successful lock */
-			ret = 1;
-			break;
-		}
-		elapsed = end.tv_sec + end.tv_usec/1000000.0 - 
-			( start.tv_sec + start.tv_usec/1000000.0 );
-		/* 
-		   The check for elapsed < 0 is to deal with the unlikely event 
-		   that after the loop is entered the system time is set forward
-		   past the loop's end time.  This would otherwise result in a 
-		   (practically) infinite loop.
-		*/
-		if( timed_out || elapsed >= timeout || elapsed < 0 )
-		{
-			if ( timed_out == 0 && force )
-			{
-				/*
-				  Timed out and force was specified - attempt to
-				  remove stale lock and try a final time
-				*/
-				(void)unlink( lockfile );
-				timed_out = 1;
-				continue;
-			}
-			else
-			{
-				/*
-				  Timed out and final try was unsuccessful or 
-				  force was not specified
-				*/
-				debug( 1, L"acquire_lock_file: timed out "
-					   L"trying to obtain lockfile %s using "
-					   L"linkfile %s", lockfile, linkfile );
-				break;
-			}
-		}
-		nanosleep( &pollint, NULL );
-	} while( gettimeofday( &end, NULL ) == 0 );
-  done:
-	/* The linkfile is not needed once the lockfile has been created */
-	(void)unlink( linkfile );
-	free( linkfile );
-	return ret;
-}
 
 void common_handle_winch( int signal )
 {

common.h

@@ -616,16 +616,6 @@ wchar_t *unescape( const wchar_t * in,
 bool unescape_string( wcstring &str, 
                   int escape_special );
 
-/**
-   Attempt to acquire a lock based on a lockfile, waiting LOCKPOLLINTERVAL 
-   milliseconds between polls and timing out after timeout seconds, 
-   thereafter forcibly attempting to obtain the lock if force is non-zero.
-   Returns 1 on success, 0 on failure.
-   To release the lock the lockfile must be unlinked.
-   A unique temporary file named by appending characters to the lockfile name 
-   is used; any pre-existing file of the same name is subject to deletion.
-*/
-int acquire_lock_file( const char *lockfile, const int timeout, int force );
 
 /** 
     Returns the width of the terminal window, so that not all

exec.cpp

@@ -198,6 +198,7 @@ void close_unused_internal_pipes( io_data_t *io )
  */
 static char *get_interpreter( const char *command, char *interpreter, size_t buff_size )
 {
+    // OK to not use CLO_EXEC here because this is only called after fork
 	int fd = open( command, O_RDONLY );
 	if( fd >= 0 )
 	{
@@ -248,6 +249,7 @@ static void safe_launch_process( process_t *p, const char *actual_cmd, char **ar
 	   /bin/sh if encountered. This is a weird predecessor to the shebang
 	   that is still sometimes used since it is supported on Windows.
 	*/
+    /* OK to not use CLO_EXEC here because this is called after fork and the file is immediately closed */
 	int fd = open(actual_cmd, O_RDONLY);
 	if (fd >= 0)
 	{
@@ -414,6 +416,7 @@ static void io_untransmogrify( io_data_t * in, io_data_t *out )
 */
 static io_data_t *io_transmogrify( io_data_t * in )
 {
+    ASSERT_IS_MAIN_THREAD();
 	if( !in )
 		return 0;
 	
@@ -876,6 +879,7 @@ void exec( parser_t &parser, job_t *j )
 							
 							case IO_FILE:
 							{
+                                /* Do not set CLO_EXEC because child needs access */
 								builtin_stdin=wopen( in->filename,
                                               in->param2.flags, OPEN_MASK );
 								if( builtin_stdin == -1 )

fish.cpp

@@ -327,12 +327,16 @@ int main( int argc, char **argv )
 				int fd;
 				wchar_t *rel_filename, *abs_filename;
 
+                
 				if( ( fd = open(file, O_RDONLY) ) == -1 )
 				{
 					wperror( L"open" );
 					return 1;
 				}
                 
+                // OK to not do this atomically since we cannot have gone multithreaded yet
+                set_cloexec(fd);
+                
 				if( *(argv+my_optind))
 				{
                     wcstring sb;

fish_pager.cpp

@@ -1006,6 +1006,7 @@ static void init( int mangle_descriptors, int out )
 		close(1);
 		close(0);
 
+        /* OK to not use CLO_EXEC here because fish_pager is single threaded */
 		if( (in = open( ttyname(2), O_RDWR )) != -1 )
 		{
 			if( dup2( 2, 1 ) == -1 )

fishd.cpp

@@ -73,6 +73,15 @@ time the original barrier request was sent have been received.
 #include "path.h"
 #include "print_help.h"
 
+#ifndef HOST_NAME_MAX
+/**
+ Maximum length of hostname return. It is ok if this is too short,
+ getting the actual hostname is not critical, so long as the string
+ is unique in the filesystem namespace.
+ */
+#define HOST_NAME_MAX 255
+#endif
+
 /**
    Maximum length of socket filename
 */
@@ -188,6 +197,252 @@ static void handle_term( int signal )
 }
 
 
+/**
+ Writes a pid_t in decimal representation to str.
+ str must contain sufficient space.
+ The conservatively approximate maximum number of characters a pid_t will 
+ represent is given by: (int)(0.31 * sizeof(pid_t) + CHAR_BIT + 1)
+ Returns the length of the string
+ */
+static int sprint_pid_t( pid_t pid, char *str )
+{
+	int len, i = 0;
+	int dig;
+    
+	/* Store digits in reverse order into string */
+	while( pid != 0 )
+	{
+		dig = pid % 10;
+		str[i] = '0' + dig;
+		pid = ( pid - dig ) / 10;
+		i++;
+	}
+	len = i;
+	/* Reverse digits */
+	i /= 2;
+	while( i )
+	{
+		i--;
+		dig = str[i];
+		str[i] = str[len -  1 - i];
+		str[len - 1 - i] = dig;
+	}
+	return len;
+}
+
+
+
+/**
+ Writes a pseudo-random number (between one and maxlen) of pseudo-random
+ digits into str.
+ str must point to an allocated buffer of size of at least maxlen chars.
+ Returns the number of digits written.
+ Since the randomness in part depends on machine time it has _some_ extra
+ strength but still not enough for use in concurrent locking schemes on a
+ single machine because gettimeofday may not return a different value on
+ consecutive calls when:
+ a) the OS does not support fine enough resolution
+ b) the OS is running on an SMP machine.
+ Additionally, gettimeofday errors are ignored.
+ Excludes chars other than digits since ANSI C only guarantees that digits
+ are consecutive.
+ */
+static int sprint_rand_digits( char *str, int maxlen )
+{
+	int i, max;
+	struct timeval tv;
+	
+	/* 
+     Seed the pseudo-random generator based on time - this assumes
+     that consecutive calls to gettimeofday will return different values
+     and ignores errors returned by gettimeofday.
+     Cast to unsigned so that wrapping occurs on overflow as per ANSI C.
+     */
+	(void)gettimeofday( &tv, NULL );
+	srand( (unsigned int)tv.tv_sec + (unsigned int)tv.tv_usec * 1000000UL );
+	max = 1 + (maxlen - 1) * (rand() / (RAND_MAX + 1.0));
+	for( i = 0; i < max; i++ )
+	{
+		str[i] = '0' + 10 * (rand() / (RAND_MAX + 1.0));
+	}
+	return i;
+}
+
+
+/**
+ Generate a filename unique in an NFS namespace by creating a copy of str and
+ appending .{hostname}.{pid} to it.  If gethostname() fails then a pseudo-
+ random string is substituted for {hostname} - the randomness of the string
+ should be strong enough across different machines.  The main assumption 
+ though is that gethostname will not fail and this is just a "safe enough"
+ fallback.
+ The memory returned should be freed using free().
+ */
+static char *gen_unique_nfs_filename( const char *filename )
+{
+	int pidlen, hnlen, orglen = strlen( filename );
+	char hostname[HOST_NAME_MAX + 1];
+	char *newname;
+	
+	if ( gethostname( hostname, HOST_NAME_MAX + 1 ) == 0 )
+	{
+		hnlen = strlen( hostname );
+	}
+	else
+	{
+		hnlen = sprint_rand_digits( hostname, HOST_NAME_MAX );
+		hostname[hnlen] = '\0';
+	}
+	newname = (char *)malloc( orglen + 1 /* period */ + hnlen + 1 /* period */ +
+                             /* max possible pid size: 0.31 ~= log(10)2 */
+                             (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1)
+                             + 1 /* '\0' */ );
+	
+	if ( newname == NULL )
+	{
+		debug( 1, L"gen_unique_nfs_filename: %s", strerror( errno ) );
+		return newname;
+	}
+	memcpy( newname, filename, orglen );
+	newname[orglen] = '.';
+	memcpy( newname + orglen + 1, hostname, hnlen );
+	newname[orglen + 1 + hnlen] = '.';
+	pidlen = sprint_pid_t( getpid(), newname + orglen + 1 + hnlen + 1 );
+	newname[orglen + 1 + hnlen + 1 + pidlen] = '\0';
+    /*	debug( 1, L"gen_unique_nfs_filename returning with: newname = \"%s\"; "
+     L"HOST_NAME_MAX = %d; hnlen = %d; orglen = %d; "
+     L"sizeof(pid_t) = %d; maxpiddigits = %d; malloc'd size: %d", 
+     newname, (int)HOST_NAME_MAX, hnlen, orglen, 
+     (int)sizeof(pid_t), 
+     (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1),
+     (int)(orglen + 1 + hnlen + 1 + 
+     (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1) + 1) ); */
+	return newname;
+}
+
+
+/**
+ The number of milliseconds to wait between polls when attempting to acquire 
+ a lockfile
+ */
+#define LOCKPOLLINTERVAL 10
+
+/**
+ Attempt to acquire a lock based on a lockfile, waiting LOCKPOLLINTERVAL 
+ milliseconds between polls and timing out after timeout seconds, 
+ thereafter forcibly attempting to obtain the lock if force is non-zero.
+ Returns 1 on success, 0 on failure.
+ To release the lock the lockfile must be unlinked.
+ A unique temporary file named by appending characters to the lockfile name 
+ is used; any pre-existing file of the same name is subject to deletion.
+ */
+static int acquire_lock_file( const char *lockfile, const int timeout, int force )
+{
+	int fd, timed_out = 0;
+	int ret = 0; /* early exit returns failure */
+	struct timespec pollint;
+	struct timeval start, end;
+	double elapsed;
+	struct stat statbuf;
+    
+	/*
+     (Re)create a unique file and check that it has one only link.
+     */
+	char *linkfile = gen_unique_nfs_filename( lockfile );
+	if( linkfile == NULL )
+	{
+		goto done;
+	}
+	(void)unlink( linkfile );
+    /* OK to not use CLO_EXEC here because fishd is single threaded */
+	if( ( fd = open( linkfile, O_CREAT|O_RDONLY, 0600 ) ) == -1 )
+	{
+		debug( 1, L"acquire_lock_file: open: %s", strerror( errno ) );
+		goto done;
+	}
+	/*
+     Don't need to check exit status of close on read-only file descriptors
+     */
+	close( fd );
+	if( stat( linkfile, &statbuf ) != 0 )
+	{
+		debug( 1, L"acquire_lock_file: stat: %s", strerror( errno ) );
+		goto done;
+	}
+	if ( statbuf.st_nlink != 1 )
+	{
+		debug( 1, L"acquire_lock_file: number of hardlinks on unique "
+              L"tmpfile is %d instead of 1.", (int)statbuf.st_nlink );
+		goto done;
+	}
+	if( gettimeofday( &start, NULL ) != 0 )
+	{
+		debug( 1, L"acquire_lock_file: gettimeofday: %s", strerror( errno ) );
+		goto done;
+	}
+	end = start;
+	pollint.tv_sec = 0;
+	pollint.tv_nsec = LOCKPOLLINTERVAL * 1000000;
+	do 
+	{
+		/*
+         Try to create a hard link to the unique file from the 
+         lockfile.  This will only succeed if the lockfile does not 
+         already exist.  It is guaranteed to provide race-free 
+         semantics over NFS which the alternative of calling 
+         open(O_EXCL|O_CREAT) on the lockfile is not.  The lock 
+         succeeds if the call to link returns 0 or the link count on 
+         the unique file increases to 2.
+         */
+		if( link( linkfile, lockfile ) == 0 ||
+           ( stat( linkfile, &statbuf ) == 0 && 
+            statbuf.st_nlink == 2 ) )
+		{
+			/* Successful lock */
+			ret = 1;
+			break;
+		}
+		elapsed = end.tv_sec + end.tv_usec/1000000.0 - 
+        ( start.tv_sec + start.tv_usec/1000000.0 );
+		/* 
+         The check for elapsed < 0 is to deal with the unlikely event 
+         that after the loop is entered the system time is set forward
+         past the loop's end time.  This would otherwise result in a 
+         (practically) infinite loop.
+         */
+		if( timed_out || elapsed >= timeout || elapsed < 0 )
+		{
+			if ( timed_out == 0 && force )
+			{
+				/*
+                 Timed out and force was specified - attempt to
+                 remove stale lock and try a final time
+                 */
+				(void)unlink( lockfile );
+				timed_out = 1;
+				continue;
+			}
+			else
+			{
+				/*
+                 Timed out and final try was unsuccessful or 
+                 force was not specified
+                 */
+				debug( 1, L"acquire_lock_file: timed out "
+                      L"trying to obtain lockfile %s using "
+                      L"linkfile %s", lockfile, linkfile );
+				break;
+			}
+		}
+		nanosleep( &pollint, NULL );
+	} while( gettimeofday( &end, NULL ) == 0 );
+done:
+	/* The linkfile is not needed once the lockfile has been created */
+	(void)unlink( linkfile );
+	free( linkfile );
+	return ret;
+}
+
 /**
    Acquire the lock for the socket
    Returns the name of the lock file if successful or 
@@ -502,6 +757,7 @@ static void load_or_save( int save)
 		   save?"saving":"loading", 
 		   name );
 	
+    /* OK to not use CLO_EXEC here because fishd is single threaded */
 	fd = open( name, save?(O_CREAT | O_TRUNC | O_WRONLY):O_RDONLY, 0600);
 	
 	free( name );

history.cpp

@@ -494,7 +494,7 @@ void history_t::load_old_if_needed(void)
 	
 	if( ! filename.empty() )
 	{
-		if( ( fd = wopen( filename, O_RDONLY ) ) > 0 )
+		if( ( fd = wopen_cloexec( filename, O_RDONLY ) ) > 0 )
 		{
 			off_t len = lseek( fd, 0, SEEK_END );
 			if( len != (off_t)-1)

iothread.cpp

@@ -12,8 +12,6 @@
 #include <fcntl.h>
 #include <queue>
 
-#define VOMIT_ON_FAILURE(a) do { if (0 != (a)) { int err = errno; fprintf(stderr, "%s failed on line %d in file %s: %d (%s). Break on debug_thread_error to debug.\n", #a, __LINE__, __FILE__, err, strerror(err)); debug_thread_error(); abort(); }} while (0)
-
 #ifdef _POSIX_THREAD_THREADS_MAX
   #if _POSIX_THREAD_THREADS_MAX < 64
     #define IO_MAX_THREADS _POSIX_THREAD_THREADS_MAX

mimedb.cpp

@@ -196,6 +196,7 @@ char *my_strdup( const char *s )
 */
 static const char * search_ini( const char *filename, const char *match )
 {
+    /* OK to not use CLO_EXEC here because mimedb is single threaded */
 	FILE *f = fopen( filename, "r" );
 	char buf[4096];
 	int len=strlen(match);
@@ -584,6 +585,7 @@ static char *get_description( const char *mimetype )
 		return 0;
 	}
 
+    /* OK to not use CLO_EXEC here because mimedb is single threaded */
 	fd = open( fn.c_str(), O_RDONLY );
 
 //	fprintf( stderr, "%s\n", fn );

parser.cpp

@@ -680,9 +680,7 @@ void parser_t::destroy()
 {
 	if( profile )
 	{
-		/*
+		/* Save profiling information. OK to not use CLO_EXEC here because this is called while fish is dying (and hence will not fork) */
-		  Save profiling information
-		*/
 		FILE *f = fopen( profile, "w" );
 		if( !f )
 		{

postfork.cpp

@@ -169,6 +169,7 @@ static int handle_child_io( io_data_t *io )
 
 			case IO_FILE:
 			{
+                // Here we definitely do not want to set CLO_EXEC because our child needs access
 				if( (tmp=wopen( io->filename,
 						io->param2.flags, OPEN_MASK ) )==-1 )
 				{

wutil.cpp

@@ -71,6 +71,8 @@ void wutil_destroy()
 {
 }
 
+static pthread_mutex_t readdir_lock = PTHREAD_MUTEX_INITIALIZER;
+
 bool wreaddir_resolving(DIR *dir, const std::wstring &dir_path, std::wstring &out_name, bool *out_is_dir)
 {
     struct dirent *d = readdir( dir );
@@ -144,8 +146,39 @@ int wchdir( const wcstring &dir )
 
 FILE *wfopen(const wcstring &path, const char *mode)
 {
-	cstring tmp = wcs2string(path);
+    int permissions = 0, options = 0;
-	return fopen(tmp.c_str(), mode);
+    switch (*mode++) {
+        case 'r':
+            permissions = O_RDONLY;
+            break;
+        case 'w':
+            permissions = O_WRONLY;
+            options = O_CREAT | O_TRUNC;
+            break;
+        case 'a':
+            permissions = O_WRONLY;
+            options = O_CREAT | O_APPEND;
+            break;
+        default:
+            errno = EINVAL;
+            return NULL;
+            break;
+    }
+    /* Skip binary */
+    if (*mode == 'b')
+        mode++;
+    
+    /* Consider append option */
+    if (*mode == '+')
+        permissions = O_RDWR;
+    
+    int fd = wopen_cloexec(path, permissions | options, 0666);
+    if (fd < 0)
+        return NULL;
+    FILE *result = fdopen(fd, mode);
+    if (result == NULL)
+        close(fd);
+    return result;
 }
 
 FILE *wfreopen(const wcstring &path, const char *mode, FILE *stream)
@@ -154,24 +187,41 @@ FILE *wfreopen(const wcstring &path, const char *mode, FILE *stream)
     return freopen(tmp.c_str(), mode, stream);
 }
 
-int wopen(const wcstring &pathname, int flags, ...)
+bool set_cloexec(int fd) {
+    int flags = fcntl(fd, F_GETFD, 0);
+    if (flags < 0) {
+        return false;
+    } else if (flags & FD_CLOEXEC) {
+        return true;
+    } else {
+        return fcntl(fd, F_SETFD, flags | FD_CLOEXEC) >= 0;
+    }
+}
+
+static int wopen_internal(const wcstring &pathname, int flags, mode_t mode, bool cloexec)
 {
     cstring tmp = wcs2string(pathname);
-	int res=-1;
+    int fd = ::open(tmp.c_str(), flags, mode);
-	va_list argp;	
+    if (cloexec && fd >= 0 && ! set_cloexec(fd)) {
-    if( ! (flags & O_CREAT) )
+        close(fd);
+        fd = -1;
+    }
+    return fd;
+    
+}
+int wopen(const wcstring &pathname, int flags, mode_t mode)
 {
-        res = open(tmp.c_str(), flags);
+    // off the main thread, always use wopen_cloexec
+    ASSERT_IS_MAIN_THREAD();
+    return wopen_internal(pathname, flags, mode, false);
 }
-    else
+
+int wopen_cloexec(const wcstring &pathname, int flags, mode_t mode)
 {
-        va_start( argp, flags );
+    return wopen_internal(pathname, flags, mode, true);
-        res = open(tmp.c_str(), flags, va_arg(argp, int) );
-        va_end( argp );
-    }
-    return res;
 }
 
+
 int wcreat(const wcstring &pathname, mode_t mode)
 {
     cstring tmp = wcs2string(pathname);

wutil.h

@@ -42,19 +42,23 @@ void wutil_init();
 void wutil_destroy();
 
 /**
-   Wide character version of fopen().
+   Wide character version of fopen(). This sets CLO_EXEC.
 */
 FILE *wfopen(const wcstring &path, const char *mode);
 
+/** Sets CLO_EXEC on a given fd */
+bool set_cloexec(int fd);
+
 /**
    Wide character version of freopen().
 */
 FILE *wfreopen(const wcstring &path, const char *mode, FILE *stream);
 
-/**
+/** Wide character version of open(). */
-   Wide character version of open().
+int wopen(const wcstring &pathname, int flags, mode_t mode = 0);
-*/
+
-int wopen(const wcstring &pathname, int flags, ...);
+/** Wide character version of open() that also sets the close-on-exec flag (atomically when possible). */
+int wopen_cloexec(const wcstring &pathname, int flags, mode_t mode = 0);
 
 /**
    Wide character version of creat().
@@ -62,9 +66,7 @@ int wopen(const wcstring &pathname, int flags, ...);
 int wcreat(const wcstring &pathname, mode_t mode);
 
 
-/**
+/** Wide character version of opendir(). Note that opendir() is guaranteed to set close-on-exec by POSIX (hooray). */
-   Wide character version of opendir().
-*/
 DIR *wopendir(const wcstring &name);
 
 /**

xdgmime.cpp

@@ -441,6 +441,7 @@ xdg_mime_get_mime_type_for_file (const char *file_name)
   if (data == NULL)
     return XDG_MIME_TYPE_UNKNOWN;
 
+  /* OK to not use CLO_EXEC here because mimedb is single threaded */
   file = fopen (file_name, "r");
   if (file == NULL)
     {

xdgmimealias.cpp

@@ -124,6 +124,7 @@ _xdg_mime_alias_read_from_file (XdgAliasList *list,
   char line[255];
   int alloc;
 
+  /* OK to not use CLO_EXEC here because mimedb is single threaded */
   file = fopen (file_name, "r");
 
   if (file == NULL)

xdgmimeglob.cpp

@@ -447,6 +447,7 @@ _xdg_mime_glob_read_from_file (XdgGlobHash *glob_hash,
   FILE *glob_file;
   char line[255];
 
+  /* OK to not use CLO_EXEC here because mimedb is single threaded */
   glob_file = fopen (file_name, "r");
 
   if (glob_file == NULL)

xdgmimemagic.cpp

@@ -766,6 +766,7 @@ _xdg_mime_magic_read_from_file (XdgMimeMagic *mime_magic,
   FILE *magic_file;
   char header[12];
 
+  /* OK to not use CLO_EXEC here because mimedb is single threaded */
   magic_file = fopen (file_name, "r");
 
   if (magic_file == NULL)

xdgmimeparent.cpp

@@ -130,6 +130,7 @@ _xdg_mime_parent_read_from_file (XdgParentList *list,
   int i, alloc;
   XdgMimeParents *entry;
 
+  /* OK to not use CLO_EXEC here because mimedb is single threaded */
   file = fopen (file_name, "r");
 
   if (file == NULL)