pcs 98/05/09 08:26:31
Modified: src/main http_main.c Log: Start cleaning up the Win32 code. This cleans up the code which spawns child processes. The changes are -- remove an unbounded sprintf() -- always check for errors at all stages, and log an error message -- if an error occurs in creating a child, die, but make sure all already created children a killed correctly -- remove assert()'s Revision Changes Path 1.345 +64 -13 apache-1.3/src/main/http_main.c Index: http_main.c =================================================================== RCS file: /export/home/cvs/apache-1.3/src/main/http_main.c,v retrieving revision 1.344 retrieving revision 1.345 diff -u -r1.344 -r1.345 --- http_main.c 1998/05/09 13:23:56 1.344 +++ http_main.c 1998/05/09 15:26:29 1.345 @@ -4882,20 +4882,40 @@ exit(0); } /* standalone_main */ +/* Spawn a child Apache process. The child process has the command + * line arguments from argc and argv[], plus a -Z argument giving the + * name of an event. The child should open and poll or wait on this + * event. When it is signalled, the child should die. prefix is a + * prefix string for the event name. + * + * The child_num argument on entry contains a serial number for this + * child (used to create a unique event name). On exit, this number + * will have been incremented by one, ready for the next call. + * + * On exit, the value pointed to be *ev will contain the event created + * to signal the new child process. + * + * The return value is the handle to the child process if successful, + * else -1. If -1 is returned the error will already have been logged + * by ap_log_error(). + */ int create_event_and_spawn(int argc, char **argv, event **ev, int *child_num, char *prefix) { - int pid = getpid(); char buf[40], mod[200]; int i, rv; char **pass_argv = (char **) alloca(sizeof(char *) * (argc + 3)); - sprintf(buf, "%s_%d", prefix, ++(*child_num)); + ap_snprintf(buf, sizeof(buf), "%s_%d", prefix, ++(*child_num)); _flushall(); *ev = CreateEvent(NULL, TRUE, FALSE, buf); + if (!*ev) { + ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL, + "could not create event for child process"); + return -1; + } APD2("create_event_and_spawn(): created process kill event %s", buf); - ap_assert(*ev); pass_argv[0] = argv[0]; pass_argv[1] = "-Z"; pass_argv[2] = buf; @@ -4903,12 +4923,27 @@ pass_argv[i + 2] = argv[i]; } pass_argv[argc + 2] = NULL; - - GetModuleFileName(NULL, mod, 200); + rv = GetModuleFileName(NULL, mod, sizeof(mod)); + if (rv == sizeof(mod)) { + /* mod[] was not big enough for our pathname */ + ap_log_error(APLOG_MARK, APLOG_ERR, NULL, + "Internal error: path to Apache process too long"); + return -1; + } + if (rv == 0) { + ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL, + "GetModuleFileName() for current process"); + return -1; + } rv = spawnv(_P_NOWAIT, mod, pass_argv); + if (rv == -1) { + ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL, + "spawn of child process %s failed", mod); + return -1; + } - return (rv); + return rv; } /********************************************************************** @@ -4941,18 +4976,24 @@ APD4("cleanup_processes: removed child in slot %d handle %d, max=%d", position, handle, *processes); } -void create_process(HANDLE *handles, HANDLE *events, int *processes, int *child_num, char *kill_event_name, int argc, char **argv) +int create_process(HANDLE *handles, HANDLE *events, int *processes, int *child_num, char *kill_event_name, int argc, char **argv) { int i = *processes; HANDLE kill_event; + int child_handle; - handles[i] = (HANDLE)create_event_and_spawn(argc, argv, &kill_event, child_num, kill_event_name); - ap_assert(handles[i] >= 0); + child_handle = create_event_and_spawn(argc, argv, &kill_event, child_num, kill_event_name); + if (child_handle <= 0) { + return -1; + } + handles[i] = (HANDLE)child_handle; events[i] = kill_event; (*processes)++; APD4("create_processes: created child in slot %d handle %d, max=%d", (*processes)-1, handles[(*processes)-1], *processes); + + return 0; } int master_main(int argc, char **argv) @@ -4970,7 +5011,7 @@ HANDLE process_kill_events[MAX_PROCESSES]; int current_live_processes = 0; /* number of child process we know about */ int processes_to_create = 0; /* number of child processes to create */ - pool *pparent; /* pool for the parent process. Cleaned on each restart */ + pool *pparent = NULL; /* pool for the parent process. Cleaned on each restart */ nchild = 1; /* only allowed one child process for current generation */ processes_to_create = nchild; @@ -4989,9 +5030,13 @@ start_mutex = ap_create_mutex(buf); ev = (event **) alloca(sizeof(event *) * nchild); child = (int *) alloca(sizeof(int) * (nchild+1)); + while (processes_to_create--) { service_set_status(SERVICE_START_PENDING); - create_process(process_handles, process_kill_events, ¤t_live_processes, &child_num, buf, argc, argv); + if (create_process(process_handles, process_kill_events, + ¤t_live_processes, &child_num, buf, argc, argv) < 0) { + goto die_now; + } } service_set_status(SERVICE_RUNNING); @@ -5025,7 +5070,10 @@ /* Shouldn't happen, but better safe than sorry */ ap_log_error(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, server_conf, "master_main: no child processes alive! creating one"); - create_process(process_handles, process_kill_events, ¤t_live_processes, &child_num, buf, argc, argv); + if (create_process(process_handles, process_kill_events, + ¤t_live_processes, &child_num, buf, argc, argv) < 0) { + goto die_now; + } if (processes_to_create) { processes_to_create--; } @@ -5103,6 +5151,7 @@ APD2("*** main process shutdown, processes=%d ***", current_live_processes); +die_now: CloseHandle(signal_event); tmstart = time(NULL); @@ -5125,7 +5174,9 @@ } service_set_status(SERVICE_STOPPED); - ap_destroy_pool(pparent); + if (pparent) { + ap_destroy_pool(pparent); + } ap_destroy_mutex(start_mutex); return (0);