On Tue, Apr 10, 2012 at 12:05 AM, wrote: > Author: wrowe > Date: Tue Apr 10 04:05:23 2012 > New Revision: 1311569 > > URL: http://svn.apache.org/viewvc?rev=1311569&view=rev > Log: > Introduce FcgidWin32PreventOrphans directive on Windows to use OS > Job Control Objects to terminate all running fcgi's when the worker > process has been abruptly terminated. > > [wrowe: I've changed the volume level on some error levels, moved > the rv into the error log schema, and moved all commentary over to > the actual conf assignment function.  With the exception of style > cleanup, this remains almost entirely Thangaraj's creation.  Also > have added simple docs.] > > PR: 51078 > Submitted by: Thangaraj AntonyCrouse Is anyone opposed to changing the directive from _NO_ARGS to _FLAG? (a couple of more comments are below) I'm happy to do the tweaks after confirmation. > Modified: >    httpd/mod_fcgid/trunk/CHANGES-FCGID >    httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml >    httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c >    httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.h >    httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c >    httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c >    httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c > > Modified: httpd/mod_fcgid/trunk/CHANGES-FCGID > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/CHANGES-FCGID?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] (original) > +++ httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] Tue Apr 10 04:05:23 2012 > @@ -1,6 +1,11 @@ >                                                          -*- coding: utf-8 -*- >  Changes with mod_fcgid 2.3.7 > > +  *) Introduce FcgidWin32PreventOrphans directive on Windows to use OS > +     Job Control Objects to terminate all running fcgi's when the worker > +     process has been abruptly terminated. PR: 51078 > +     [Thangaraj AntonyCrouse ] > + >   *) Periodically clean out the brigades which are pulling in the request >      body for handoff to the fcgid child. PR: 51749 >      [Dominic Benson ] > > Modified: httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml (original) > +++ httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml Tue Apr 10 04:05:23 2012 > @@ -1124,6 +1124,22 @@ >   > >   > +    FcgidWin32PreventOrphans > +    Job Control orphan prevention for fcgi workers. > +    FcgidWin32PreventOrphans > +    [disabled] > +    server config > +     > +      

Uses Job Control Objects on Windows, only, to enforce shutdown of all > +      fcgi processes created by the httpd worker when the httpd worker has been > +      terminated. Processes terminated in this way do not have the opportunity > +      to clean up gracefully, complete pending disk writes, or similar closure > +      transactions, therefore this behavior is experimental and disabled, by > +      default.

> +    
> +  
> + > +   >     FcgidWrapper >     The CGI wrapper setting >     FcgidWrapper command [ suffix ] [ virtual ] > > Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c (original) > +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c Tue Apr 10 04:05:23 2012 > @@ -749,6 +749,71 @@ const char *set_access_authoritative(cmd >     return NULL; >  } > > + > +#ifdef WIN32 > +/* FcgidWin32PreventOrphans > + * > + *   When Apache process gets recycled or shutdown abruptly, CGI processes > + *   spawned by mod_fcgid will get orphaned. Orphaning happens mostly when > + *   Apache worker threads take more than 30 seconds to exit gracefully. > + * > + * Apache when run as windows service during shutdown/restart of service > + * process (master/parent) will terminate child httpd process within 30 > + * seconds (refer \server\mpm\winnt\mpm_winnt.c:master_main() > + * int timeout = 30000; ~line#1142), therefore if Apache worker threads > + * are too busy to react to Master's graceful exit signal within 30 seconds > + * mod_fcgid cleanup routines will not get invoked (refer child_main() > + * \server\mpm\winnt\child.c: apr_pool_destroy(pchild); ~line#2275) > + * thereby orphaning all mod_fcgid spwaned CGI processes. Therefore we utilize > + * Win32 JobObjects to clean up child processes automatically so that CGI > + * processes are force-killed by win32 during abnormal mod_fcgid termination. > + * > + */ > +const char *set_win32_prevent_process_orphans(cmd_parms *cmd, void *dummy, > +                                              char *arg) > +{ > +    server_rec *s = cmd->server; > +    fcgid_server_conf *config = ap_get_module_config(s->module_config, > +                                                     &fcgid_module); > + > +    if (config != NULL && config->hJobObjectForAutoCleanup == NULL) > +    { > +        /* Create Win32 job object to prevent CGI process oprhaning > +         */ > +        JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = { 0 }; > +        config->hJobObjectForAutoCleanup = CreateJobObject(NULL, NULL); > + > +        if (config->hJobObjectForAutoCleanup == NULL) { > +            ap_log_error(APLOG_MARK, APLOG_ERR, apr_get_os_error(), NULL, > +                         "mod_fcgid: Error enabling CGI process orphan " > +                         "prevention: unable to create job object."); > +            return NULL; > +        } > + > +        /* Set job info so that all spawned CGI processes are associated > +         * with mod_fcgid > +         */ > +        job_info.BasicLimitInformation.LimitFlags > +            = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; > +        if (SetInformationJobObject(config->hJobObjectForAutoCleanup, > +                                    JobObjectExtendedLimitInformation, > +                                    &job_info, sizeof(job_info)) == 0) { > +            ap_log_error(APLOG_MARK, APLOG_ERR, apr_get_os_error(), NULL, > +                         "mod_fcgid: Error enabling CGI process orphan " > +                         "prevention: unable to set job object information."); > +            CloseHandle(config->hJobObjectForAutoCleanup); > +            config->hJobObjectForAutoCleanup = NULL; > +            return NULL; > +        } > + > +        ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, > +                     "mod_fcgid: Enabled CGI process orphan prevention flag."); > +    } > + > +    return NULL; > +} > +#endif /* WIN32*/ > + >  fcgid_cmd_conf *get_access_info(request_rec * r, int *authoritative) >  { >     fcgid_dir_conf *config = > > Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.h > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.h?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.h (original) > +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.h Tue Apr 10 04:05:23 2012 > @@ -71,6 +71,10 @@ typedef struct { >     int termination_score; >     int time_score; >     int zombie_scan_interval; > +#ifdef WIN32 > +    /* FcgidWin32PreventOrphans - Win32 CGI processes automatic cleanup */ > +    HANDLE hJobObjectForAutoCleanup; > +#endif >     /* global or vhost >      * scalar values have corresponding _set field to aid merging >      */ > > Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c (original) > +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c Tue Apr 10 04:05:23 2012 > @@ -264,6 +264,7 @@ int procmgr_must_exit() >  apr_status_t procmgr_stop_procmgr(void *server) >  { >     apr_status_t status; > +    fcgid_server_conf *conf; > >     /* Tell the world to die */ >     g_must_exit = 1; > @@ -281,6 +282,14 @@ apr_status_t procmgr_stop_procmgr(void * >         } >     } > > +    /* Cleanup the Job object if present */ > +    conf = ap_get_module_config(((server_rec*)server)->module_config, > +                                &fcgid_module); > + > +    if (conf != NULL && conf->hJobObjectForAutoCleanup != NULL) { > +        CloseHandle(conf->hJobObjectForAutoCleanup); > +    } > + Isn't it more idiomatic to register a cleanup for the job object rather than explicitly checking for whether or not it exists in different code? >     if (g_wakeup_thread) >         return apr_thread_join(&status, g_wakeup_thread); > > > Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c (original) > +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c Tue Apr 10 04:05:23 2012 > @@ -63,6 +63,7 @@ apr_status_t proc_spawn_process(const ch >  { >     HANDLE *finish_event, listen_handle; >     SECURITY_ATTRIBUTES SecurityAttributes; > +    fcgid_server_conf *sconf; >     apr_procattr_t *proc_attr; >     apr_status_t rv; >     apr_file_t *file; > @@ -177,9 +178,31 @@ apr_status_t proc_spawn_process(const ch >     if (rv != APR_SUCCESS) { >         ap_log_error(APLOG_MARK, APLOG_ERR, rv, procinfo->main_server, >                      "mod_fcgid: can't run %s", wargv[0]); > +        return rv; >     } > > -    return rv; > +    /* FcgidWin32PreventOrphans feature */ > +    sconf = ap_get_module_config(procinfo->main_server->module_config, > +                                 &fcgid_module); > +    if (sconf == NULL) { > +        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, procinfo->main_server, > +                     "mod_fcgid: missing server configuration record"); > +        return APR_SUCCESS; > +    } Let's just crash here if sconf is NULL. Agreed? > + > +    if (sconf->hJobObjectForAutoCleanup != NULL) > +    { > +        /* Associate cgi process to current process */ > +        if (AssignProcessToJobObject(sconf->hJobObjectForAutoCleanup, > +                                     procnode->proc_id.hproc) == 0) { > +            ap_log_error(APLOG_MARK, APLOG_WARNING, apr_get_os_error(), > +                         procinfo->main_server, > +                         "mod_fcgid: unable to assign child process to " > +                         "job object"); > +        } > +    } > + > +    return APR_SUCCESS; >  } > >  apr_status_t proc_kill_gracefully(fcgid_procnode *procnode, > > Modified: httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c > URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c?rev=1311569&r1=1311568&r2=1311569&view=diff > ============================================================================== > --- httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c (original) > +++ httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c Tue Apr 10 04:05:23 2012 > @@ -802,6 +802,11 @@ fcgid_init(apr_pool_t * config_pool, apr >     return APR_SUCCESS; >  } > > +#ifdef WIN32 > +const char *set_win32_prevent_process_orphans(cmd_parms *cmd, void *dummy, > +                                              char *arg); > +#endif > + >  static const command_rec fcgid_cmds[] = { >     AP_INIT_TAKE1("FcgidAccessChecker", set_access_info, NULL, >                   ACCESS_CONF | OR_FILEINFO, > @@ -895,6 +900,12 @@ static const command_rec fcgid_cmds[] = >     AP_INIT_TAKE1("FcgidZombieScanInterval", set_zombie_scan_interval, NULL, >                   RSRC_CONF, >                   "scan interval for zombie process"), > +#ifdef WIN32 > +    AP_INIT_NO_ARGS("FcgidWin32PreventOrphans", > +                    set_win32_prevent_process_orphans, NULL, RSRC_CONF, > +                    "Prevented fcgi process orphaning during Apache worker " > +                    "abrupt shutdowns [see documentation]"), > +#endif > >     /* The following directives are all deprecated in favor >      * of a consistent use of the Fcgid prefix. > > -- Born in Roswell... married an alien...