On Tue, Apr 10, 2012 at 12:05 AM, <[email protected]> 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 <thangaraj gmail.com>
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 <thangaraj gmail.com>] > + > *) Periodically clean out the brigades which are pulling in the request > body for handoff to the fcgid child. PR: 51749 > [Dominic Benson <dominic.benson thirdlight.com>] > > 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 @@ > </directivesynopsis> > > <directivesynopsis> > + <name>FcgidWin32PreventOrphans</name> > + <description>Job Control orphan prevention for fcgi > workers.</description> > + <syntax>FcgidWin32PreventOrphans</syntax> > + <default>[disabled]</default> > + <contextlist><context>server config</context></contextlist> > + <usage> > + <p>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.</p> > + </usage> > + </directivesynopsis> > + > + <directivesynopsis> > <name>FcgidWrapper</name> > <description>The CGI wrapper setting</description> > <syntax>FcgidWrapper <em>command</em> [ <em>suffix</em> ] [ virtual > ]</syntax> > > 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...
