From 909d24cde6acf87587370355d7a9cbc7dc18435c Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 28 Feb 2012 15:11:46 -0800 Subject: [PATCH] More work on improving interaction between fork and pthreads. Added null_terminated_array_t class. --- common.cpp | 4 + common.h | 106 ++++++++++- env.cpp | 132 +++++++------- env.h | 5 +- exec.cpp | 480 +++++++------------------------------------------ exec.h | 3 + fish.cpp | 6 +- fish_tests.cpp | 4 +- input.cpp | 2 +- postfork.cpp | 314 ++++++++++++++++++++++++++++++++ postfork.h | 4 + proc.cpp | 25 --- proc.h | 17 +- reader.cpp | 2 +- signal.cpp | 2 +- 15 files changed, 576 insertions(+), 530 deletions(-) diff --git a/common.cpp b/common.cpp index f0903261f..049cdf342 100644 --- a/common.cpp +++ b/common.cpp @@ -2041,6 +2041,10 @@ double timef() return (double)tv.tv_sec + 0.000001*tv.tv_usec; } +void exit_without_destructors(int code) { + _exit(code); +} + void append_path_component(wcstring &path, const wcstring &component) { size_t len = path.size(); diff --git a/common.h b/common.h index f3f4d577f..b1186df60 100644 --- a/common.h +++ b/common.h @@ -77,6 +77,8 @@ typedef std::vector wcstring_list_t; */ #define VOMIT_ON_FAILURE(a) do { if (0 != (a)) { int err = errno; fprintf(stderr, "%s failed on line %d in file %s: %d (%s)\n", #a, __LINE__, __FILE__, err, strerror(err)); abort(); }} while (0) +/** Exits without invoking destructors (via _exit), useful for code after fork. This would be a good candidate for __noreturn attribute. */ +void exit_without_destructors(int code); /** Save the shell mode on startup so we can restore them on exit @@ -132,7 +134,7 @@ extern const wchar_t *program_name; int exit_read_count;char exit_read_buff; \ show_stackframe(); \ exit_read_count=read( 0, &exit_read_buff, 1 ); \ - exit( 1 ); \ + exit_without_destructors( 1 ); \ } \ @@ -301,8 +303,110 @@ wcstring to_string(const T &x) { return stream.str(); } +/* Helper class for managing a null-terminated array of null-terminated strings (of some char type) */ +template +class null_terminated_array_t { + CharType_t **array; + + typedef std::basic_string string_t; + typedef std::vector string_list_t; + + void swap(null_terminated_array_t &him) { std::swap(array, him.array); } + + /* Silly function to get the length of a null terminated array of...something */ + template + static size_t count_not_null(const T *arr) { + size_t len; + for (len=0; arr[len] != T(0); len++) + ; + return len; + } + + size_t size() const { + return count_not_null(array); + } + + void free(void) { + if (array != NULL) { + for (size_t i = 0; array[i] != NULL; i++) { + delete [] array[i]; + } + delete [] array; + array = NULL; + } + } + + public: + null_terminated_array_t() : array(NULL) { } + ~null_terminated_array_t() { this->free(); } + + /** operator=. Notice the pass-by-value parameter. */ + null_terminated_array_t& operator=(null_terminated_array_t rhs) { + if (this != &rhs) + this->swap(rhs); + return *this; + } + + /* Copy constructor. */ + null_terminated_array_t(const null_terminated_array_t &him) { + this->set(him.array); + } + + void set(const string_list_t &argv) { + /* Get rid of the old argv */ + this->free(); + + /* Allocate our null-terminated array of null-terminated strings */ + size_t i, count = argv.size(); + this->array = new CharType_t * [count + 1]; + for (i=0; i < count; i++) { + const string_t &str = argv.at(i); + this->array[i] = new CharType_t [1 + str.size()]; + std::copy(str.begin(), str.end(), this->array[i]); + this->array[i][str.size()] = CharType_t(0); + } + this->array[count] = NULL; + } + + void set(const CharType_t * const *new_array) { + if (new_array == array) + return; + + /* Get rid of the old argv */ + this->free(); + + /* Copy the new one */ + if (new_array) { + size_t i, count = count_not_null(new_array); + this->array = new CharType_t * [count + 1]; + for (i=0; i < count; i++) { + size_t len = count_not_null(new_array[i]); + this->array[i] = new CharType_t [1 + len]; + std::copy(new_array[i], new_array[i] + len, this->array[i]); + this->array[i][len] = CharType_t(0); + } + this->array[count] = NULL; + } + } + + CharType_t **get() { return array; } + const CharType_t * const *get() const { return array; } + + string_list_t to_list() const { + string_list_t lst; + if (array != NULL) { + size_t count = this->size(); + lst.reserve(count); + lst.insert(lst.end(), array, array + count); + } + return lst; + } + +}; + bool is_forked_child(); +/* Basic scoped lock class */ class scoped_lock { pthread_mutex_t *lock_obj; bool locked; diff --git a/env.cpp b/env.cpp index ed558f193..42e4ecd5e 100644 --- a/env.cpp +++ b/env.cpp @@ -41,7 +41,6 @@ #include - #include "fallback.h" #include "util.h" @@ -93,6 +92,7 @@ struct var_entry_t var_entry_t() : exportv(false) { } }; +typedef std::map var_table_t; /** Struct representing one level in the function variable stack @@ -102,7 +102,7 @@ struct env_node_t /** Variable table */ - std::map env; + var_table_t env; /** Does this node imply a new variable scope? If yes, all non-global variables below this one in the stack are @@ -145,7 +145,7 @@ static env_node_t *global_env = 0; /** Table for global variables */ -static std::map *global; +static var_table_t *global; /** Table of variables that may not be set using the set command. @@ -171,7 +171,7 @@ static bool is_electric(const wcstring &key) /** Exported variable array used by execv */ -static char **export_arr=0; +static null_terminated_array_t export_array; /** Buffer used for storing string contents for export_arr @@ -183,7 +183,7 @@ static buffer_t export_buffer; Flag for checking if we need to regenerate the exported variable array */ -static int has_changed = 1; +static bool has_changed = true; /** This string is used to store the value of dynamically @@ -377,7 +377,7 @@ static void universal_callback( int type, if( str ) { - has_changed=1; + has_changed=true; event_t ev = event_t::variable_event(name); ev.arguments.reset(new wcstring_list_t()); @@ -682,19 +682,18 @@ void env_destroy() env_electric.clear(); - std::map::iterator iter; + var_table_t::iterator iter; for (iter = global->begin(); iter != global->end(); ++iter) { var_entry_t *entry = iter->second; if( entry->exportv ) { - has_changed = 1; + has_changed = true; } delete entry; } delete top; - free( export_arr ); } /** @@ -709,7 +708,7 @@ static env_node_t *env_get_node( const wcstring &key ) while( env != 0 ) { - std::map::const_iterator result = env->env.find( key ); + var_table_t::const_iterator result = env->env.find( key ); if ( result != env->env.end() ) { @@ -735,8 +734,8 @@ int env_set( const wchar_t *key, int var_mode ) { env_node_t *node = NULL; - int has_changed_old = has_changed; - int has_changed_new = 0; + bool has_changed_old = has_changed; + bool has_changed_new = false; var_entry_t *e=0; int done=0; @@ -818,7 +817,7 @@ int env_set( const wchar_t *key, if( node ) { - std::map::iterator result = node->env.find(key); + var_table_t::iterator result = node->env.find(key); if ( result != node->env.end() ) { e = result->second; } @@ -828,7 +827,7 @@ int env_set( const wchar_t *key, if( e->exportv ) { - has_changed_new = 1; + has_changed_new = true; } } @@ -895,7 +894,7 @@ int env_set( const wchar_t *key, if( !done ) { var_entry_t *old_entry = NULL; - std::map::iterator result = node->env.find(key); + var_table_t::iterator result = node->env.find(key); if ( result != node->env.end() ) { old_entry = result->second; @@ -910,7 +909,7 @@ int env_set( const wchar_t *key, if( (var_mode & ENV_EXPORT) || entry->exportv ) { entry->exportv = !!(var_mode & ENV_EXPORT); - has_changed_new = 1; + has_changed_new = true; } } else @@ -920,7 +919,7 @@ int env_set( const wchar_t *key, if( var_mode & ENV_EXPORT) { entry->exportv = 1; - has_changed_new = 1; + has_changed_new = true; } else { @@ -981,14 +980,14 @@ static int try_remove( env_node_t *n, return 0; } - std::map::iterator result = n->env.find( key ); + var_table_t::iterator result = n->env.find( key ); if ( result != n->env.end() ) { var_entry_t *v = result->second; if( v->exportv ) { - has_changed = 1; + has_changed = true; } n->env.erase(result); @@ -1136,7 +1135,7 @@ env_var_t env_get_string( const wcstring &key ) while( env != 0 ) { - std::map::iterator result = env->env.find(key); + var_table_t::iterator result = env->env.find(key); if ( result != env->env.end() ) { res = result->second; @@ -1250,7 +1249,7 @@ const wchar_t *env_get( const wchar_t *key ) while( env != 0 ) { - std::map::iterator result = env->env.find(key); + var_table_t::iterator result = env->env.find(key); if ( result != env->env.end() ) { res = result->second; @@ -1326,7 +1325,7 @@ int env_exist( const wchar_t *key, int mode ) while( env != 0 ) { - std::map::iterator result = env->env.find( key ); + var_table_t::iterator result = env->env.find( key ); if ( result != env->env.end() ) { res = result->second; @@ -1416,7 +1415,7 @@ void env_pop() for( i=0; locale_variable[i]; i++ ) { - std::map::iterator result = killme->env.find( locale_variable[i] ); + var_table_t::iterator result = killme->env.find( locale_variable[i] ); if ( result != killme->env.end() ) { locale_changed = 1; @@ -1431,13 +1430,13 @@ void env_pop() top = top->next; - std::map::iterator iter; + var_table_t::iterator iter; for (iter = killme->env.begin(); iter != killme->env.end(); ++iter) { var_entry_t *entry = iter->second; if( entry->exportv ) { - has_changed = 1; + has_changed = true; } delete entry; } @@ -1459,9 +1458,9 @@ void env_pop() /** Function used with to insert keys of one table into a set::set */ -static void add_key_to_string_set(const std::map &envs, std::set &strSet) +static void add_key_to_string_set(const var_table_t &envs, std::set &strSet) { - std::map::const_iterator iter; + var_table_t::const_iterator iter; for (iter = envs.begin(); iter != envs.end(); ++iter) { var_entry_t *e = iter->second; @@ -1548,17 +1547,17 @@ wcstring_list_t env_get_names( int flags ) Get list of all exported variables */ -static void get_exported2( const env_node_t *n, std::map &h ) +static void get_exported( const env_node_t *n, std::map &h ) { if( !n ) return; if( n->new_scope ) - get_exported2( global_env, h ); + get_exported( global_env, h ); else - get_exported2( n->next, h ); + get_exported( n->next, h ); - std::map::const_iterator iter; + var_table_t::const_iterator iter; for (iter = n->env.begin(); iter != n->env.end(); ++iter) { const wcstring &key = iter->first; @@ -1570,10 +1569,7 @@ static void get_exported2( const env_node_t *n, std::map &h } } -/** - Function used by env_export_arr to iterate over map of variables -*/ -static void export_func2(std::map &envs, buffer_t* out) +static void export_func(const std::map &envs, std::vector &out) { std::map::const_iterator iter; for (iter = envs.begin(); iter != envs.end(); ++iter) @@ -1587,20 +1583,23 @@ static void export_func2(std::map &envs, buffer_t* out) *pos = ':'; pos++; } - int nil = 0; - - b_append( out, ks, strlen(ks) ); - b_append( out, "=", 1 ); - b_append( out, vs, strlen(vs) ); - b_append( out, &nil, 1 ); + + /* Put a string on the vector */ + out.push_back(std::string()); + std::string &str = out.back(); + + /* Append our environment variable data to it */ + str.append(ks); + str.append("="); + str.append(vs); free(ks); free(vs); - } + } } -char **env_export_arr( int recalc ) -{ +static void update_export_array_if_necessary(bool recalc) { + ASSERT_IS_MAIN_THREAD(); if( recalc && !proc_had_barrier) { proc_had_barrier=1; @@ -1610,13 +1609,11 @@ char **env_export_arr( int recalc ) if( has_changed ) { std::map vals; - int prev_was_null=1; - int pos=0; size_t i; debug( 4, L"env_export_arr() recalc" ); - get_exported2( top, vals ); + get_exported( top, vals ); wcstring_list_t uni; env_universal_get_names2( uni, 1, 0 ); @@ -1632,28 +1629,27 @@ char **env_export_arr( int recalc ) } } - - export_buffer.used=0; - - export_func2(vals, &export_buffer ); - - export_arr = (char **)realloc( export_arr, - sizeof(char *)*(1 + vals.size())); //add 1 for null termination - - for( i=0; i local_export_buffer; + export_func(vals, local_export_buffer ); + export_array.set(local_export_buffer); + has_changed=false; } - return export_arr; + +} + +char **env_export_arr( int recalc ) +{ + ASSERT_IS_MAIN_THREAD(); + update_export_array_if_necessary(recalc != 0); + return export_array.get(); +} + +void env_export_arr(bool recalc, null_terminated_array_t &output) +{ + ASSERT_IS_MAIN_THREAD(); + update_export_array_if_necessary(recalc != 0); + output = export_array; } env_vars::env_vars(const wchar_t * const *keys) diff --git a/env.h b/env.h index d3fbf44b8..a286df6d8 100644 --- a/env.h +++ b/env.h @@ -157,10 +157,9 @@ void env_push( int new_scope ); */ void env_pop(); -/** - Returns an array containing all exported variables in a format suitable for execv. -*/ +/** Returns an array containing all exported variables in a format suitable for execv. */ char **env_export_arr( int recalc ); +void env_export_arr(bool recalc, null_terminated_array_t &result); /** Returns all variable names. diff --git a/exec.cpp b/exec.cpp index 78d64695b..9aa58ac65 100644 --- a/exec.cpp +++ b/exec.cpp @@ -35,6 +35,7 @@ #include "fallback.h" #include "util.h" #include "iothread.h" +#include "postfork.h" #include "common.h" #include "wutil.h" @@ -66,26 +67,6 @@ */ #define FILE_ERROR _( L"An error occurred while redirecting file '%ls'" ) -/** - file redirection clobbering error message -*/ -#define NOCLOB_ERROR _( L"The file '%ls' already exists" ) - -/** - fork error message -*/ -#define FORK_ERROR _( L"Could not create child process - exiting" ) - -/** - The number of times to try to call fork() before giving up -*/ -#define FORK_LAPS 5 - -/** - The number of nanoseconds to sleep between attempts to call fork() -*/ -#define FORK_SLEEP_TIME 1000000 - /** Base open mode to pass to calls to open */ @@ -95,25 +76,27 @@ List of all pipes used by internal pipes. These must be closed in many situations in order to make sure that stray fds aren't lying around. + + Note this is used after fork, so we must not do anything that may allocate memory. Hopefully methods like open_fds.at() don't. */ -static std::vector open_fds; - -static int set_child_group( job_t *j, process_t *p, int print_errors ); +static std::vector open_fds; +// Called in a forked child static void exec_write_and_exit( int fd, char *buff, size_t count, int status ) { if( write_loop(fd, buff, count) == -1 ) { debug( 0, WRITE_ERROR); wperror( L"write" ); - exit(status); + exit_without_destructors(status); } - exit( status ); + exit_without_destructors( status ); } void exec_close( int fd ) { + /* This may be called in a child of fork(), so don't allocate memory */ if( fd < 0 ) { debug( 0, L"Called close on invalid file descriptor " ); @@ -130,8 +113,10 @@ void exec_close( int fd ) } } - /* Maybe remove this form our set of open fds */ - open_fds.erase(std::remove(open_fds.begin(), open_fds.end(), fd), open_fds.end()); + /* Maybe remove this from our set of open fds */ + if (fd < (int)open_fds.size()) { + open_fds[fd] = false; + } } int exec_pipe( int fd[2]) @@ -149,8 +134,12 @@ int exec_pipe( int fd[2]) debug( 4, L"Created pipe using fds %d and %d", fd[0], fd[1]); - open_fds.push_back(fd[0]); - open_fds.push_back(fd[1]); + int max_fd = std::max(fd[0], fd[1]); + if ((int)open_fds.size() <= max_fd) { + open_fds.resize(max_fd + 1, false); + } + open_fds.at(fd[0]) = true; + open_fds.at(fd[1]) = true; return res; } @@ -187,256 +176,22 @@ static int use_fd_in_pipe( int fd, io_data_t *io ) \param io the list of io redirections for this job. Pipes mentioned here should not be closed. */ -static void close_unused_internal_pipes( io_data_t *io ) +void close_unused_internal_pipes( io_data_t *io ) { /* A call to exec_close will modify open_fds, so be careful how we walk */ for (size_t i=0; i < open_fds.size(); i++) { - int fd = open_fds.at(i); - if( !use_fd_in_pipe( fd, io) ) - { - debug( 4, L"Close fd %d, used in other context", fd ); - exec_close( fd ); - i--; + if (open_fds[i]) { + int fd = (int)i; + if( !use_fd_in_pipe( fd, io) ) + { + debug( 4, L"Close fd %d, used in other context", fd ); + exec_close( fd ); + i--; + } } } } -/** - Make sure the fd used by this redirection is not used by i.e. a pipe. -*/ -void free_fd( io_data_t *io, int fd ) -{ - if( !io ) - return; - - if( ( io->io_mode == IO_PIPE ) || ( io->io_mode == IO_BUFFER ) ) - { - int i; - for( i=0; i<2; i++ ) - { - if(io->param1.pipe_fd[i] == fd ) - { - while(1) - { - if( (io->param1.pipe_fd[i] = dup(fd)) == -1) - { - if( errno != EINTR ) - { - debug( 1, - FD_ERROR, - fd ); - wperror( L"dup" ); - FATAL_EXIT(); - } - } - else - { - break; - } - } - } - } - } - free_fd( io->next, fd ); -} - -/** - Set up a childs io redirections. Should only be called by - setup_child_process(). Does the following: First it closes any open - file descriptors not related to the child by calling - close_unused_internal_pipes() and closing the universal variable - server file descriptor. It then goes on to perform all the - redirections described by \c io. - - \param io the list of IO redirections for the child - - \return 0 on sucess, -1 on failiure -*/ -static int handle_child_io( io_data_t *io ) -{ - - close_unused_internal_pipes( io ); - - for( ; io; io=io->next ) - { - int tmp; - - if( io->io_mode == IO_FD && io->fd == io->param1.old_fd ) - { - continue; - } - - if( io->fd > 2 ) - { - /* - Make sure the fd used by this redirection is not used by e.g. a pipe. - */ - free_fd( io, io->fd ); - } - - switch( io->io_mode ) - { - case IO_CLOSE: - { - if( close(io->fd) ) - { - debug( 0, _(L"Failed to close file descriptor %d"), io->fd ); - wperror( L"close" ); - } - break; - } - - case IO_FILE: - { - if( (tmp=wopen( io->filename, - io->param2.flags, OPEN_MASK ) )==-1 ) - { - if( ( io->param2.flags & O_EXCL ) && - ( errno ==EEXIST ) ) - { - debug( 1, - NOCLOB_ERROR, - io->filename.c_str() ); - } - else - { - debug( 1, - FILE_ERROR, - io->filename.c_str() ); - - wperror( L"open" ); - } - - return -1; - } - else if( tmp != io->fd) - { - /* - This call will sometimes fail, but that is ok, - this is just a precausion. - */ - close(io->fd); - - if(dup2( tmp, io->fd ) == -1 ) - { - debug( 1, - FD_ERROR, - io->fd ); - wperror( L"dup2" ); - return -1; - } - exec_close( tmp ); - } - break; - } - - case IO_FD: - { - /* - This call will sometimes fail, but that is ok, - this is just a precausion. - */ - close(io->fd); - - if( dup2( io->param1.old_fd, io->fd ) == -1 ) - { - debug( 1, - FD_ERROR, - io->fd ); - wperror( L"dup2" ); - return -1; - } - break; - } - - case IO_BUFFER: - case IO_PIPE: - { - int write_pipe; - - write_pipe = !io->is_input; -/* - debug( 0, - L"%ls %ls on fd %d (%d %d)", - write_pipe?L"write":L"read", - (io->io_mode == IO_BUFFER)?L"buffer":L"pipe", - io->fd, - io->param1.pipe_fd[0], - io->param1.pipe_fd[1]); -*/ - if( dup2( io->param1.pipe_fd[write_pipe], io->fd ) != io->fd ) - { - debug( 1, PIPE_ERROR ); - wperror( L"dup2" ); - return -1; - } - - if( write_pipe ) - { - exec_close( io->param1.pipe_fd[0]); - exec_close( io->param1.pipe_fd[1]); - } - else - { - exec_close( io->param1.pipe_fd[0] ); - } - break; - } - - } - } - - return 0; - -} - -/** - Initialize a new child process. This should be called right away - after forking in the child process. If job control is enabled for - this job, the process is put in the process group of the job, all - signal handlers are reset, signals are unblocked (this function may - only be called inside the exec function, which blocks all signals), - and all IO redirections and other file descriptor actions are - performed. - - \param j the job to set up the IO for - \param p the child process to set up - - \return 0 on sucess, -1 on failiure. When this function returns, - signals are always unblocked. On failiure, signal handlers, io - redirections and process group of the process is undefined. -*/ -static int setup_child_process( job_t *j, process_t *p ) -{ - int res=0; - - if( p ) - { - res = set_child_group( j, p, 1 ); - } - - if( !res ) - { - res = handle_child_io( j->io ); - if( p != 0 && res ) - { - exit( 1 ); - } - } - - /* Set the handling for job control signals back to the default. */ - if( !res ) - { - signal_reset_handlers(); - } - - /* Remove all signal blocks */ - signal_unblock(); - - return res; - -} - /** Returns the interpreter for the specified script. Returns false if file is not a script with a shebang. @@ -470,7 +225,7 @@ static bool get_interpreter( const wcstring &file, wcstring &interpreter ) } } - + /** This function is executed by the child process created by a call to fork(). It should be called after \c setup_child_process. It calls @@ -579,7 +334,7 @@ static void launch_process( process_t *p ) debug( 0, L"Try running the command again with fewer arguments."); - exit(STATUS_EXEC_FAIL); + exit_without_destructors(STATUS_EXEC_FAIL); break; } @@ -589,7 +344,7 @@ static void launch_process( process_t *p ) wperror(L"exec"); debug(0, L"The file '%ls' is marked as an executable but could not be run by the operating system.", p->actual_cmd); - exit(STATUS_EXEC_FAIL); + exit_without_destructors(STATUS_EXEC_FAIL); } case ENOENT: @@ -604,13 +359,13 @@ static void launch_process( process_t *p ) debug(0, L"The file '%ls' or a script or ELF interpreter does not exist, or a shared library needed for file or interpreter cannot be found.", p->actual_cmd); } - exit(STATUS_EXEC_FAIL); + exit_without_destructors(STATUS_EXEC_FAIL); } case ENOMEM: { debug(0, L"Out of memory"); - exit(STATUS_EXEC_FAIL); + exit_without_destructors(STATUS_EXEC_FAIL); } default: @@ -618,7 +373,7 @@ static void launch_process( process_t *p ) wperror(L"exec"); // debug(0, L"The file '%ls' is marked as an executable but could not be run by the operating system.", p->actual_cmd); - exit(STATUS_EXEC_FAIL); + exit_without_destructors(STATUS_EXEC_FAIL); } } @@ -763,135 +518,24 @@ static void internal_exec_helper( parser_t &parser, is_block=is_block_old; } -/** - This function should be called by both the parent process and the - child right after fork() has been called. If job control is - enabled, the child is put in the jobs group, and if the child is - also in the foreground, it is also given control of the - terminal. When called in the parent process, this function may - fail, since the child might have already finished and called - exit. The parent process may safely ignore the exit status of this - call. - - Returns 0 on sucess, -1 on failiure. -*/ -static int set_child_group( job_t *j, process_t *p, int print_errors ) +/** Perform output from builtins. Called from a forked child, so don't do anything that may allocate memory, etc.. */ +static void do_builtin_io( const char *out, const char *err ) { - int res = 0; - - if( job_get_flag( j, JOB_CONTROL ) ) + size_t len; + if (out && (len = strlen(out))) { - if (!j->pgid) - { - j->pgid = p->pid; - } - - if( setpgid (p->pid, j->pgid) ) - { - if( getpgid( p->pid) != j->pgid && print_errors ) - { - debug( 1, - _( L"Could not send process %d, '%ls' in job %d, '%ls' from group %d to group %d" ), - p->pid, - p->argv0(), - j->job_id, - j->command_cstr(), - getpgid( p->pid), - j->pgid ); - wperror( L"setpgid" ); - res = -1; - } - } - } - else - { - j->pgid = getpid(); - } - - if( job_get_flag( j, JOB_TERMINAL ) && job_get_flag( j, JOB_FOREGROUND ) ) - { - if( tcsetpgrp (0, j->pgid) && print_errors ) - { - debug( 1, _( L"Could not send job %d ('%ls') to foreground" ), - j->job_id, - j->command_cstr() ); - wperror( L"tcsetpgrp" ); - res = -1; - } - } - - return res; -} - -/** - This function is a wrapper around fork. If the fork calls fails - with EAGAIN, it is retried FORK_LAPS times, with a very slight - delay between each lap. If fork fails even then, the process will - exit with an error message. -*/ -static pid_t exec_fork() -{ - ASSERT_IS_MAIN_THREAD(); - - /* Make sure we have no outstanding threads before we fork. This is a pretty sketchy thing to do here, both because exec.cpp shouldn't have to know about iothreads, and because the completion handlers may do unexpected things. */ - iothread_drain_all(); - - pid_t pid; - struct timespec pollint; - int i; - - for( i=0; i= 0) - { - return pid; - } - - if( errno != EAGAIN ) - { - break; - } - - pollint.tv_sec = 0; - pollint.tv_nsec = FORK_SLEEP_TIME; - - /* - Don't sleep on the final lap - sleeping might change the - value of errno, which will break the error reporting below. - */ - if( i != FORK_LAPS-1 ) - { - nanosleep( &pollint, NULL ); - } - } - - debug( 0, FORK_ERROR ); - wperror (L"fork"); - FATAL_EXIT(); -} - - -/** - Perform output from builtins - */ -static void do_builtin_io( const wchar_t *out, const wchar_t *err ) -{ - - if( out ) - { - if( fwprintf( stdout, L"%ls", out ) == -1 || fflush( stdout ) == EOF ) + if (write_loop(STDOUT_FILENO, out, len) == -1) { debug( 0, L"Error while writing to stdout" ); - wperror( L"fwprintf" ); + wperror( L"write_loop" ); show_stackframe(); } } - if( err ) + if (err && (len = strlen(err))) { - if( fwprintf( stderr, L"%ls", err ) == -1 || fflush( stderr ) == EOF ) + if (write_loop(STDERR_FILENO, err, len) == -1) { /* Can't really show any error message here, since stderr is @@ -899,8 +543,7 @@ static void do_builtin_io( const wchar_t *out, const wchar_t *err ) */ } } - -} +} void exec( parser_t &parser, job_t *j ) @@ -1049,15 +692,15 @@ void exec( parser_t &parser, job_t *j ) if( needs_keepalive ) { - keepalive.pid = exec_fork(); - + /* Call fork. No need to wait for threads since our use is confined and simple. */ + keepalive.pid = execute_fork(false); if( keepalive.pid == 0 ) { /* Child */ keepalive.pid = getpid(); set_child_group( j, &keepalive, 1 ); pause(); - exit(0); + exit_without_destructors(0); } else { @@ -1401,8 +1044,8 @@ void exec( parser_t &parser, job_t *j ) if( io_buffer->param2.out_buffer->used != 0 ) { - pid = exec_fork(); - + /* We don't have to drain threads here because our child process is simple */ + pid = execute_fork(false); if( pid == 0 ) { @@ -1450,7 +1093,7 @@ void exec( parser_t &parser, job_t *j ) case INTERNAL_BUFFER: { - pid = exec_fork(); + pid = execute_fork(false); if( pid == 0 ) { @@ -1542,14 +1185,17 @@ void exec( parser_t &parser, job_t *j ) break; } - /* - Ok, unfortunatly, we have to do a real fork. Bummer. - */ - - pid = exec_fork(); + /* Ok, unfortunatly, we have to do a real fork. Bummer. We work hard to make sure we don't have to wait for all our threads to exit, by arranging things so that we don't have to allocate memory or do anything except system calls in the child. */ + + /* Get the strings we'll write before we fork (since they call malloc) */ + const wcstring &out = get_stdout_buffer(), &err = get_stderr_buffer(); + char *outbuff = wcs2str(out.c_str()), *errbuff = wcs2str(err.c_str()); + + fflush(stdout); + fflush(stderr); + pid = execute_fork(false); if( pid == 0 ) { - /* This is the child process. Setup redirections, print correct output to stdout and stderr, and @@ -1557,13 +1203,17 @@ void exec( parser_t &parser, job_t *j ) */ p->pid = getpid(); setup_child_process( j, p ); - const wcstring &out = get_stdout_buffer(), &err = get_stderr_buffer(); - do_builtin_io( out.empty() ? NULL : out.c_str(), err.empty() ? NULL : err.c_str() ); - exit( p->status ); + do_builtin_io(outbuff, errbuff); + exit_without_destructors( p->status ); } else { + + /* Free the strings in the parent */ + free(outbuff); + free(errbuff); + /* This is the parent process. Store away information on the child, and possibly give @@ -1580,7 +1230,7 @@ void exec( parser_t &parser, job_t *j ) case EXTERNAL: { - pid = exec_fork(); + pid = execute_fork(true /* must drain threads */); if( pid == 0 ) { /* diff --git a/exec.h b/exec.h index ddbe2dcb0..5d882a002 100644 --- a/exec.h +++ b/exec.h @@ -70,4 +70,7 @@ void exec_close( int fd ); */ int exec_pipe( int fd[2]); +/* Close all fds in open_fds. This is called from postfork.cpp */ +void close_unused_internal_pipes( io_data_t *io ); + #endif diff --git a/fish.cpp b/fish.cpp index 67448e00a..12011651e 100644 --- a/fish.cpp +++ b/fish.cpp @@ -188,7 +188,7 @@ static int fish_parse_opt( int argc, char **argv, const char **cmd_ptr ) else { debug( 0, _(L"Invalid value '%s' for debug level switch"), optarg ); - exit(1); + exit_without_destructors(1); } break; } @@ -229,12 +229,12 @@ static int fish_parse_opt( int argc, char **argv, const char **cmd_ptr ) _(L"%s, version %s\n"), PACKAGE_NAME, PACKAGE_VERSION ); - exit( 0 ); + exit_without_destructors( 0 ); } case '?': { - exit( 1 ); + exit_without_destructors( 1 ); } } diff --git a/fish_tests.cpp b/fish_tests.cpp index f1a3cb7fe..c8f72e75c 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -552,7 +552,7 @@ void perf_complete() str[0]=c; reader_set_buffer( str, 0 ); - complete( str, out ); + complete( str, out, COMPLETE_DEFAULT, NULL ); matches += out.size(); out.clear(); @@ -572,7 +572,7 @@ void perf_complete() reader_set_buffer( str, 0 ); - complete( str, out ); + complete( str, out, COMPLETE_DEFAULT, NULL ); matches += out.size(); out.clear(); diff --git a/input.cpp b/input.cpp index 9fc2a0ffa..d16fb9618 100644 --- a/input.cpp +++ b/input.cpp @@ -317,7 +317,7 @@ int input_init() if( setupterm( 0, STDOUT_FILENO, 0) == ERR ) { debug( 0, _( L"Could not set up terminal" ) ); - exit(1); + exit_without_destructors(1); } const env_var_t term = env_get_string( L"TERM" ); assert(! term.missing()); diff --git a/postfork.cpp b/postfork.cpp index 2ea88b23d..0be315625 100644 --- a/postfork.cpp +++ b/postfork.cpp @@ -3,7 +3,34 @@ Functions that we may safely call after fork(). */ +#include +#include "signal.h" #include "postfork.h" +#include "iothread.h" +#include "exec.h" + + +/** The number of times to try to call fork() before giving up */ +#define FORK_LAPS 5 + +/** The number of nanoseconds to sleep between attempts to call fork() */ +#define FORK_SLEEP_TIME 1000000 + +/** Base open mode to pass to calls to open */ +#define OPEN_MASK 0666 + +/** fork error message */ +#define FORK_ERROR _( L"Could not create child process - exiting" ) + +/** file redirection clobbering error message */ +#define NOCLOB_ERROR _( L"The file '%ls' already exists" ) + +/** file redirection error message */ +#define FILE_ERROR _( L"An error occurred while redirecting file '%ls'" ) + +/** file descriptor redirection error message */ +#define FD_ERROR _( L"An error occurred while redirecting file descriptor %d" ) + /** This function should be called by both the parent process and the @@ -17,6 +44,8 @@ Returns 0 on sucess, -1 on failiure. */ + +// PCA These calls to debug are rather sketchy because they may allocate memory. Fortunately they only occur if an error occurs. int set_child_group( job_t *j, process_t *p, int print_errors ) { int res = 0; @@ -66,3 +95,288 @@ int set_child_group( job_t *j, process_t *p, int print_errors ) return res; } +/** Make sure the fd used by this redirection is not used by i.e. a pipe. */ +static void free_fd( io_data_t *io, int fd ) +{ + if( !io ) + return; + + if( ( io->io_mode == IO_PIPE ) || ( io->io_mode == IO_BUFFER ) ) + { + int i; + for( i=0; i<2; i++ ) + { + if(io->param1.pipe_fd[i] == fd ) + { + while(1) + { + if( (io->param1.pipe_fd[i] = dup(fd)) == -1) + { + if( errno != EINTR ) + { + debug( 1, + FD_ERROR, + fd ); + wperror( L"dup" ); + FATAL_EXIT(); + } + } + else + { + break; + } + } + } + } + } + free_fd( io->next, fd ); +} + + +/** + Set up a childs io redirections. Should only be called by + setup_child_process(). Does the following: First it closes any open + file descriptors not related to the child by calling + close_unused_internal_pipes() and closing the universal variable + server file descriptor. It then goes on to perform all the + redirections described by \c io. + + \param io the list of IO redirections for the child + + \return 0 on sucess, -1 on failiure +*/ +static int handle_child_io( io_data_t *io ) +{ + + close_unused_internal_pipes( io ); + + for( ; io; io=io->next ) + { + int tmp; + + if( io->io_mode == IO_FD && io->fd == io->param1.old_fd ) + { + continue; + } + + if( io->fd > 2 ) + { + /* + Make sure the fd used by this redirection is not used by e.g. a pipe. + */ + free_fd( io, io->fd ); + } + + switch( io->io_mode ) + { + case IO_CLOSE: + { + if( close(io->fd) ) + { + debug( 0, _(L"Failed to close file descriptor %d"), io->fd ); + wperror( L"close" ); + } + break; + } + + case IO_FILE: + { + if( (tmp=wopen( io->filename, + io->param2.flags, OPEN_MASK ) )==-1 ) + { + if( ( io->param2.flags & O_EXCL ) && + ( errno ==EEXIST ) ) + { + debug( 1, + NOCLOB_ERROR, + io->filename.c_str() ); + } + else + { + debug( 1, + FILE_ERROR, + io->filename.c_str() ); + + wperror( L"open" ); + } + + return -1; + } + else if( tmp != io->fd) + { + /* + This call will sometimes fail, but that is ok, + this is just a precausion. + */ + close(io->fd); + + if(dup2( tmp, io->fd ) == -1 ) + { + debug( 1, + FD_ERROR, + io->fd ); + wperror( L"dup2" ); + return -1; + } + exec_close( tmp ); + } + break; + } + + case IO_FD: + { + /* + This call will sometimes fail, but that is ok, + this is just a precausion. + */ + close(io->fd); + + if( dup2( io->param1.old_fd, io->fd ) == -1 ) + { + debug( 1, + FD_ERROR, + io->fd ); + wperror( L"dup2" ); + return -1; + } + break; + } + + case IO_BUFFER: + case IO_PIPE: + { + int write_pipe; + + write_pipe = !io->is_input; +/* + debug( 0, + L"%ls %ls on fd %d (%d %d)", + write_pipe?L"write":L"read", + (io->io_mode == IO_BUFFER)?L"buffer":L"pipe", + io->fd, + io->param1.pipe_fd[0], + io->param1.pipe_fd[1]); +*/ + if( dup2( io->param1.pipe_fd[write_pipe], io->fd ) != io->fd ) + { + debug( 1, PIPE_ERROR ); + wperror( L"dup2" ); + return -1; + } + + if( write_pipe ) + { + exec_close( io->param1.pipe_fd[0]); + exec_close( io->param1.pipe_fd[1]); + } + else + { + exec_close( io->param1.pipe_fd[0] ); + } + break; + } + + } + } + + return 0; + +} + + +/** + Initialize a new child process. This should be called right away + after forking in the child process. If job control is enabled for + this job, the process is put in the process group of the job, all + signal handlers are reset, signals are unblocked (this function may + only be called inside the exec function, which blocks all signals), + and all IO redirections and other file descriptor actions are + performed. + + \param j the job to set up the IO for + \param p the child process to set up + + \return 0 on sucess, -1 on failiure. When this function returns, + signals are always unblocked. On failiure, signal handlers, io + redirections and process group of the process is undefined. +*/ +int setup_child_process( job_t *j, process_t *p ) +{ + int res=0; + + if( p ) + { + res = set_child_group( j, p, 1 ); + } + + if( !res ) + { + res = handle_child_io( j->io ); + if( p != 0 && res ) + { + exit_without_destructors( 1 ); + } + } + + /* Set the handling for job control signals back to the default. */ + if( !res ) + { + signal_reset_handlers(); + } + + /* Remove all signal blocks */ + signal_unblock(); + + return res; + +} + +/** + This function is a wrapper around fork. If the fork calls fails + with EAGAIN, it is retried FORK_LAPS times, with a very slight + delay between each lap. If fork fails even then, the process will + exit with an error message. +*/ +pid_t execute_fork(bool wait_for_threads_to_die) +{ + ASSERT_IS_MAIN_THREAD(); + + if (wait_for_threads_to_die) { + /* Make sure we have no outstanding threads before we fork. This is a pretty sketchy thing to do here, both because exec.cpp shouldn't have to know about iothreads, and because the completion handlers may do unexpected things. */ + iothread_drain_all(); + } + + pid_t pid; + struct timespec pollint; + int i; + + for( i=0; i= 0) + { + return pid; + } + + if( errno != EAGAIN ) + { + break; + } + + pollint.tv_sec = 0; + pollint.tv_nsec = FORK_SLEEP_TIME; + + /* + Don't sleep on the final lap - sleeping might change the + value of errno, which will break the error reporting below. + */ + if( i != FORK_LAPS-1 ) + { + nanosleep( &pollint, NULL ); + } + } + + debug( 0, FORK_ERROR ); + wperror (L"fork"); + FATAL_EXIT(); +} diff --git a/postfork.h b/postfork.h index 9eaf06e23..06364b312 100644 --- a/postfork.h +++ b/postfork.h @@ -23,4 +23,8 @@ int set_child_group( job_t *j, process_t *p, int print_errors ); int setup_child_process( job_t *j, process_t *p ); +/* Call fork(), optionally waiting until we are no longer multithreaded. If the forked child doesn't do anything that could allocate memory, take a lock, etc. (like call exec), then it's not necessary to wait for threads to die. If the forked child may do those things, it should wait for threads to die. +*/ +pid_t execute_fork(bool wait_for_threads_to_die); + #endif diff --git a/proc.cpp b/proc.cpp index 837aae19c..7546883d8 100644 --- a/proc.cpp +++ b/proc.cpp @@ -177,31 +177,6 @@ void job_free( job_t * j ) delete j; } -void process_t::free_argv(void) { - if (argv_array != NULL) { - for (size_t i = 0; argv_array[i] != NULL; i++) { - delete [] argv_array[i]; - } - delete [] argv_array; - } - argv_array = NULL; -} - -void process_t::set_argv(const wcstring_list_t &argv) { - /* Get rid of the old argv */ - free_argv(); - - /* Allocate our null-terminated array of null-terminated strings */ - size_t i, count = argv.size(); - argv_array = new wchar_t* [count + 1]; - for (i=0; i < count; i++) { - const wcstring &str = argv.at(i); - argv_array[i] = new wchar_t [1 + str.size()]; - wcscpy(argv_array[i], str.c_str()); - } - argv_array[i] = NULL; -} - void proc_destroy() { event.arguments.reset(NULL); diff --git a/proc.h b/proc.h index 41a263ead..d91cfd9b2 100644 --- a/proc.h +++ b/proc.h @@ -128,15 +128,13 @@ enum class process_t { private: - /** argv parameter for for execv, builtin_run, etc. This is allocated via new, and furthermore, each string within it is allocated via new as well. Null terminated. */ - wchar_t **argv_array; - - void free_argv(void); + + null_terminated_array_t argv_array; public: process_t() : - argv_array(NULL), + argv_array(), type(0), actual_cmd(NULL), pid(0), @@ -158,7 +156,6 @@ class process_t { if (this->next != NULL) delete this->next; - this->free_argv(); free((void *)actual_cmd); //may be NULL } @@ -171,16 +168,16 @@ class process_t /** Sets argv */ - void set_argv(const wcstring_list_t &argv); + void set_argv(const wcstring_list_t &argv) { argv_array.set(argv); } /** Returns argv */ - const wchar_t * const *get_argv(void) const { return argv_array; } + const wchar_t * const *get_argv(void) const { return argv_array.get(); } /** Returns argv[0] */ - const wchar_t *argv0(void) const { return argv_array[0]; } + const wchar_t *argv0(void) const { return argv_array.get()[0]; } /** Returns argv[idx] */ - const wchar_t *argv(size_t idx) const { return argv_array[idx]; } + const wchar_t *argv(size_t idx) const { return argv_array.get()[idx]; } /** actual command to pass to exec in case of EXTERNAL or INTERNAL_EXEC. malloc'd! */ const wchar_t *actual_cmd; diff --git a/reader.cpp b/reader.cpp index aa2a44fb9..b2881e967 100644 --- a/reader.cpp +++ b/reader.cpp @@ -1744,7 +1744,7 @@ static void reader_interactive_init() debug( 1, _( L"Couldn't grab control of terminal" ) ); wperror( L"tcsetpgrp" ); - exit(1); + exit_without_destructors(1); } common_handle_winch(0); diff --git a/signal.cpp b/signal.cpp index 023fa2203..83a4b13eb 100644 --- a/signal.cpp +++ b/signal.cpp @@ -595,7 +595,7 @@ void signal_set_handlers() if( sigaction( SIGCHLD, &act, 0) ) { wperror( L"sigaction" ); - exit(1); + exit_without_destructors(1); } }