On Tuesday 29 September 2009 4:20:49 pm you wrote:
> On Tue, Sep 29, 2009 at 4:59 PM, Ricardo Cantu <[email protected]> wrote:
> > On Tuesday 29 September 2009 2:31:21 pm Jeff Trawick wrote:
> > > ZombieScanInterval (leave alone until processes can be reaped
> >
> > differently)
> > Working on a patch for this one. Don't want to duplicate work, so let me
> > know
> > if anybody else is working on this.
>
> not me
>
> I hope that, for Unix, processes can be reaped as with the MPMs: instead of
> asking if a specific pid has exited (for each pid in the list), ask if any
> pid has exited and if so find it in the list and handle.
>
Well, here it is. My patch to reap the children when they exit rather than
check the list for zombies. Before I take out the old logic for the zombie
scan I would like to hear some input on the code.
basically,
apr_proc_other_child_register() - to register a callback when child exits.
sigaction(SIGCHLD, &sa, NULL) - to listen for children dying.
apr_proc_other_child_refresh_all(APR_OC_REASON_RESTART) - called when SIGCHLD
received so callback will be called on the correct registered child.
fcgid_child_maint - The callback. Cleans up the various lists and prints out
log info.
And that's it.
Index: modules/fcgid/fcgid_proc_unix.c
===================================================================
--- modules/fcgid/fcgid_proc_unix.c (revision 820820)
+++ modules/fcgid/fcgid_proc_unix.c (working copy)
@@ -58,6 +58,92 @@
static int g_process_counter = 0;
static apr_pool_t *g_inode_cginame_map = NULL;
+static void
+move_idle_to_free(server_rec * main_server, fcgid_procnode * procnode)
+{
+ fcgid_procnode *previous_node, *current_node, *next_node;
+ fcgid_procnode *free_list_header, *proc_table;
+
+ proc_table = proctable_get_table_array();
+ previous_node = proctable_get_idle_list();
+ free_list_header = proctable_get_free_list();
+
+ safe_lock(main_server);
+ current_node = &proc_table[previous_node->next_index];
+ while (current_node != proc_table) {
+ next_node = &proc_table[current_node->next_index];
+
+ if (procnode == current_node) {
+ /* Unlink from idle list */
+ previous_node->next_index = current_node->next_index;
+
+ /* Link to free list */
+ current_node->next_index = free_list_header->next_index;
+ free_list_header->next_index = current_node - proc_table;
+
+ break;
+ } else
+ previous_node = current_node;
+
+ current_node = next_node;
+ }
+ safe_unlock(main_server);
+}
+
+static void fcgid_child_maint(int reason, void *data, apr_wait_t status)
+{
+ fcgid_procnode *procnode = data;
+ int exitcode;
+ apr_exit_why_e exitwhy;
+
+ if (WIFEXITED(status)) {
+ exitcode = WEXITSTATUS(status);
+ exitwhy = APR_PROC_EXIT;
+ } else {
+ exitcode = WTERMSIG(status);
+ exitwhy = APR_PROC_SIGNAL;
+ }
+
+ switch (reason) {
+ case APR_OC_REASON_DEATH:
+ case APR_OC_REASON_LOST:
+
+ /* Log why and how it die */
+ proc_print_exit_info(procnode, exitcode, exitwhy, procnode->main_server);
+
+ /* Register the death */
+ register_termination(procnode->main_server, procnode);
+
+ /* Destroy pool */
+ apr_pool_destroy(procnode->proc_pool);
+ procnode->proc_pool = NULL;
+
+ /* Fix lists */
+ move_idle_to_free(procnode->main_server, procnode);
+
+ if (reason == APR_OC_REASON_DEATH) {
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL,
+ "mod_fcgid: Pid: %d got APR_OC_REASON_DEATH", procnode->proc_id->pid);
+ } else {
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL,
+ "mod_fcgid: Pid: %d got APR_OC_REASON_LOST", procnode->proc_id->pid);
+ }
+ break;
+
+ case APR_OC_REASON_RESTART:
+
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL,
+ "mod_fcgid: Pid: %d got APR_OC_REASON_RESTART", procnode->proc_id->pid);
+ break;
+
+ case APR_OC_REASON_UNREGISTER:
+
+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL,
+ "mod_fcgid: Pid: %d got APR_OC_REASON_UNREGISTER", procnode->proc_id->pid);
+ break;
+ }
+}
+
static apr_status_t ap_unix_create_privileged_process(apr_proc_t *newproc,
const char *progname,
const char *const *args,
@@ -382,6 +468,10 @@
}
}
+ /* setup handler for child death */
+ apr_proc_other_child_register(procnode->proc_id, fcgid_child_maint,
+ procnode, NULL, procnode->proc_pool);
+
/* Set the (deviceid, inode) -> fastcgi path map for log */
apr_snprintf(key_name, _POSIX_PATH_MAX, "%lX%lX",
(unsigned long) procnode->inode,
@@ -457,7 +547,7 @@
apr_exit_why_e exitwhy;
rv = apr_proc_wait(procnode->proc_id, &exitcode, &exitwhy, APR_NOWAIT);
- if (rv == APR_CHILD_DONE || rv == APR_EGENERAL) {
+ if (rv != APR_CHILD_NOTDONE) {
/* Log why and how it die */
proc_print_exit_info(procnode, exitcode, exitwhy, main_server);
@@ -776,23 +866,12 @@
proc_print_exit_info(fcgid_procnode * procnode, int exitcode,
apr_exit_why_e exitwhy, server_rec * main_server)
{
- const char *cgipath = NULL;
const char *diewhy = NULL;
char signal_info[HUGE_STRING_LEN];
- char key_name[_POSIX_PATH_MAX];
int signum = exitcode;
- void* tmp;
memset(signal_info, 0, HUGE_STRING_LEN);
- /* Get the file name infomation base on inode and deviceid */
- apr_snprintf(key_name, _POSIX_PATH_MAX, "%lX%lX",
- (unsigned long) procnode->inode,
- (unsigned long) procnode->deviceid);
- apr_pool_userdata_get(&tmp, key_name,
- g_inode_cginame_map);
- cgipath = tmp;
-
/* Reasons to exit */
switch (procnode->diewhy) {
case FCGID_DIE_KILLSELF:
@@ -832,7 +911,7 @@
break;
default:
- if (APR_PROC_CHECK_CORE_DUMP(exitwhy)) {
+ if (APR_PROC_CHECK_CORE_DUMP(exitwhy) || signum == SIGSEGV) {
apr_snprintf(signal_info, HUGE_STRING_LEN - 1,
"get signal %d, possible coredump generated",
signum);
@@ -850,12 +929,7 @@
}
/* Print log now */
- if (cgipath)
- ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, main_server,
- "mod_fcgid: process %s(%" APR_PID_T_FMT ") exit(%s), %s",
- cgipath, procnode->proc_id->pid, diewhy, signal_info);
- else
- ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, main_server,
- "mod_fcgid: can't get CGI name while exiting, exitcode: %d",
- exitcode);
+ ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, main_server,
+ "mod_fcgid: process %s(%" APR_PID_T_FMT ") exit(%s), %s",
+ procnode->cgipath, procnode->proc_id->pid, diewhy, signal_info);
}
Index: modules/fcgid/fcgid_proctbl_unix.c
===================================================================
--- modules/fcgid/fcgid_proctbl_unix.c (revision 820820)
+++ modules/fcgid/fcgid_proctbl_unix.c (working copy)
@@ -200,14 +200,40 @@
return rv;
}
+void
+block_child_signal (void)
+{
+ sigset_t blocked_set;
+ sigemptyset(&blocked_set);
+
+ sigaddset(&blocked_set, SIGCHLD);
+ pthread_sigmask(SIG_SETMASK, &blocked_set, NULL);
+}
+
+void
+unblock_child_signal (void)
+{
+ sigset_t blocked_set;
+ sigemptyset(&blocked_set);
+
+ sigaddset(&blocked_set, SIGCHLD);
+ pthread_sigmask(SIG_UNBLOCK, &blocked_set, NULL);
+}
+
apr_status_t proctable_lock_table(void)
{
+ block_child_signal();
return apr_global_mutex_lock(g_sharelock);
}
apr_status_t proctable_unlock_table(void)
{
- return apr_global_mutex_unlock(g_sharelock);
+ apr_status_t status;
+
+ status = apr_global_mutex_unlock(g_sharelock);
+
+ unblock_child_signal();
+ return status;
}
fcgid_procnode *proctable_get_free_list(void)
Index: modules/fcgid/fcgid_pm_main.c
===================================================================
--- modules/fcgid/fcgid_pm_main.c (revision 820820)
+++ modules/fcgid/fcgid_pm_main.c (working copy)
@@ -19,6 +19,7 @@
#define CORE_PRIVATE
#include "httpd.h"
#include "http_config.h"
+#include "apr_strings.h"
#include "fcgid_pm.h"
#include "fcgid_pm_main.h"
@@ -487,6 +488,10 @@
/* Prepare to spawn */
procnode->deviceid = command->deviceid;
procnode->inode = command->inode;
+ apr_cpystrn(procnode->cgipath, command->cgipath, _POSIX_PATH_MAX);
+ if ((procnode->cgiNameOffset = (ap_strrchr_c(command->cgipath, '/') - command->cgipath)) < 0) {
+ procnode->cgiNameOffset = 0;
+ }
procnode->share_grp_id = command->share_grp_id;
procnode->virtualhost = command->virtualhost;
procnode->uid = command->uid;
@@ -494,6 +499,7 @@
procnode->start_time = procnode->last_active_time = apr_time_now();
procnode->requests_handled = 0;
procnode->diewhy = FCGID_DIE_KILLSELF;
+ procnode->main_server = main_server;
procnode->proc_pool = NULL;
procinfo.cgipath = command->cgipath;
@@ -561,14 +567,15 @@
/* Wait for command */
if (procmgr_peek_cmd(&command, main_server) == APR_SUCCESS) {
- if (is_spawn_allowed(main_server, &command))
+ if (is_spawn_allowed(main_server, &command)) {
fastcgi_spawn(&command, main_server, configpool);
+ }
procmgr_finish_notify(main_server);
}
/* Move matched node to error list */
- scan_idlelist_zombie(main_server);
+ //scan_idlelist_zombie(main_server);
scan_idlelist(main_server);
scan_busylist(main_server);
Index: modules/fcgid/fcgid_conf.c
===================================================================
--- modules/fcgid/fcgid_conf.c (revision 820820)
+++ modules/fcgid/fcgid_conf.c (working copy)
@@ -81,6 +81,7 @@
* config->default_init_env = NULL;
* config->pass_headers = NULL;
* config->php_fix_pathinfo_enable = 0;
+ * config->cgi_name_significant = 0;
* config->*_set = 0;
*/
config->ipc_comm_timeout = DEFAULT_IPC_COMM_TIMEOUT;
@@ -303,6 +304,23 @@
return NULL;
}
+const char *set_cgi_name_significant(cmd_parms * cmd, void *dummy,
+ const char *arg)
+{
+ server_rec *s = cmd->server;
+ fcgid_server_conf *config =
+ ap_get_module_config(s->module_config, &fcgid_module);
+ const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+ if (err != NULL) {
+ return err;
+ }
+
+ config->cgi_name_significant = atol(arg);
+ return NULL;
+ }
+
+
const char *set_socketpath(cmd_parms * cmd, void *dummy, const char *arg)
{
server_rec *s = cmd->server;
Index: modules/fcgid/fcgid_spawn_ctl.c
===================================================================
--- modules/fcgid/fcgid_spawn_ctl.c (revision 820820)
+++ modules/fcgid/fcgid_spawn_ctl.c (working copy)
@@ -17,12 +17,15 @@
#include "fcgid_spawn_ctl.h"
#include "fcgid_conf.h"
+#include "apr_strings.h"
#define REGISTER_LIFE 1
#define REGISTER_DEATH 2
struct fcgid_stat_node {
apr_ino_t inode;
dev_t deviceid;
+ char cgipath[_POSIX_PATH_MAX];
+ int cgiNameOffset;
uid_t uid;
gid_t gid;
apr_size_t share_grp_id;
@@ -57,7 +60,8 @@
&& current_node->share_grp_id == procnode->share_grp_id
&& current_node->virtualhost == procnode->virtualhost
&& current_node->uid == procnode->uid
- && current_node->gid == procnode->gid)
+ && current_node->gid == procnode->gid
+ && !(sconf->cgi_name_significant && (apr_strnatcmp (current_node->cgipath+current_node->cgiNameOffset, procnode->cgipath+procnode->cgiNameOffset) != 0)))
break;
previous_node = current_node;
}
@@ -90,6 +94,8 @@
current_node = apr_pcalloc(g_stat_pool, sizeof(*current_node));
current_node->deviceid = procnode->deviceid;
current_node->inode = procnode->inode;
+ apr_cpystrn(current_node->cgipath, procnode->cgipath, _POSIX_PATH_MAX);
+ current_node->cgiNameOffset = procnode->cgiNameOffset;
current_node->share_grp_id = procnode->share_grp_id;
current_node->virtualhost = procnode->virtualhost;
current_node->uid = procnode->uid;
@@ -107,9 +113,15 @@
}
}
+static void signal_handler (int sig)
+{
+ apr_proc_other_child_refresh_all(APR_OC_REASON_RESTART);
+}
+
void spawn_control_init(server_rec * main_server, apr_pool_t * configpool)
{
apr_status_t rv;
+ struct sigaction sa;
if ((rv = apr_pool_create(&g_stat_pool, configpool)) != APR_SUCCESS) {
/* Fatal error */
@@ -117,6 +129,15 @@
"mod_fcgid: can't create stat pool");
exit(1);
}
+
+ /* Setup child signal handler */
+ sa.sa_handler = signal_handler;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = 0;
+ if (sigaction(SIGCHLD, &sa, NULL) < 0) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, rv, main_server,
+ "mod_fcgid: sigaction(SIGCHLD) failed");
+ }
}
void
@@ -146,7 +167,12 @@
struct fcgid_stat_node *current_node;
fcgid_server_conf *sconf = ap_get_module_config(main_server->module_config,
&fcgid_module);
-
+ int cgiNameOffset;
+ if ((cgiNameOffset = (ap_strrchr_c(command->cgipath, '/') - command->cgipath)) < 0) {
+ cgiNameOffset = 0;
+ }
+ char *cgipath = command->cgipath+cgiNameOffset;
+
if (!command || !g_stat_pool)
return 1;
@@ -158,7 +184,8 @@
&& current_node->share_grp_id == command->share_grp_id
&& current_node->virtualhost == command->virtualhost
&& current_node->uid == command->uid
- && current_node->gid == command->gid)
+ && current_node->gid == command->gid
+ && !(sconf->cgi_name_significant && (apr_strnatcmp (current_node->cgipath+current_node->cgiNameOffset, cgipath) != 0)))
break;
}
@@ -225,7 +252,8 @@
&& current_node->share_grp_id == procnode->share_grp_id
&& current_node->virtualhost == procnode->virtualhost
&& current_node->uid == procnode->uid
- && current_node->gid == procnode->gid)
+ && current_node->gid == procnode->gid
+ && !(sconf->cgi_name_significant && (apr_strnatcmp (current_node->cgipath+current_node->cgiNameOffset, procnode->cgipath+procnode->cgiNameOffset) != 0)))
break;
previous_node = current_node;
}
Index: modules/fcgid/fcgid_conf.h
===================================================================
--- modules/fcgid/fcgid_conf.h (revision 820820)
+++ modules/fcgid/fcgid_conf.h (working copy)
@@ -80,6 +80,7 @@
int termination_score;
int time_score;
int zombie_scan_interval;
+ int cgi_name_significant;
/* global or vhost
* scalar values have corresponding _set field to aid merging
*/
@@ -159,6 +160,9 @@
const char *set_max_request_len(cmd_parms * cmd, void *dummy,
const char *arg);
+const char *set_cgi_name_significant(cmd_parms * cmd, void *dummy,
+ const char *arg);
+
const char *set_max_mem_request_len(cmd_parms * cmd, void *dummy,
const char *arg);
Index: modules/fcgid/fcgid_proctbl.h
===================================================================
--- modules/fcgid/fcgid_proctbl.h (revision 820820)
+++ modules/fcgid/fcgid_proctbl.h (working copy)
@@ -42,6 +42,8 @@
char socket_path[_POSIX_PATH_MAX]; /* cgi application socket path */
apr_ino_t inode; /* cgi file inode */
apr_dev_t deviceid; /* cgi file device id */
+ char cgipath[_POSIX_PATH_MAX]; /* cgi file path */
+ int cgiNameOffset; /* program name in cgipath */
gid_t gid; /* for suEXEC */
uid_t uid; /* for suEXEC */
apr_size_t share_grp_id; /* cgi wrapper share group id */
@@ -50,6 +52,7 @@
apr_time_t last_active_time; /* the time this process last active */
int requests_handled; /* number of requests process has handled */
char diewhy; /* why it die */
+ server_rec *main_server; /* server rec */
} fcgid_procnode;
/* Macros for diewhy */
Index: modules/fcgid/mod_fcgid.c
===================================================================
--- modules/fcgid/mod_fcgid.c (revision 820820)
+++ modules/fcgid/mod_fcgid.c (working copy)
@@ -684,6 +684,11 @@
AP_INIT_TAKE1("FCGIDZombieScanInterval", set_zombie_scan_interval, NULL,
RSRC_CONF,
"scan interval for zombie process"),
+ AP_INIT_TAKE1("FCGIDCGINameSignificant",
+ set_cgi_name_significant,
+ NULL, RSRC_CONF,
+ "Set 1, if program/script name makes it unique"),
+
/* The following directives are all deprecated in favor
* of a consistent use of the FCGID prefix.
Index: modules/fcgid/fcgid_bridge.c
===================================================================
--- modules/fcgid/fcgid_bridge.c (revision 820820)
+++ modules/fcgid/fcgid_bridge.c (working copy)
@@ -38,6 +38,9 @@
{
/* Scan idle list, find a node match inode, deviceid and groupid
If there is no one there, return NULL */
+
+ fcgid_server_conf *sconf = ap_get_module_config(s->module_config,
+ &fcgid_module);
fcgid_procnode *previous_node, *current_node, *next_node;
fcgid_procnode *busy_list_header, *proc_table;
apr_ino_t inode = command->inode;
@@ -46,7 +49,13 @@
gid_t gid = command->gid;
apr_size_t share_grp_id = command->share_grp_id;
const char *virtualhost = command->virtualhost;
-
+ int cgi_name_significant = sconf->cgi_name_significant;
+ int cgiNameOffset;
+ if ((cgiNameOffset = (ap_strrchr_c(command->cgipath, '/') - command->cgipath)) < 0) {
+ cgiNameOffset = 0;
+ }
+ char *cgipath = command->cgipath+cgiNameOffset;
+
proc_table = proctable_get_table_array();
previous_node = proctable_get_idle_list();
busy_list_header = proctable_get_busy_list();
@@ -60,7 +69,8 @@
&& current_node->deviceid == deviceid
&& current_node->share_grp_id == share_grp_id
&& current_node->virtualhost == virtualhost
- && current_node->uid == uid && current_node->gid == gid) {
+ && current_node->uid == uid && current_node->gid == gid
+ && !(cgi_name_significant && (apr_strnatcmp (current_node->cgipath+current_node->cgiNameOffset, cgipath) != 0))) {
/* Unlink from idle list */
previous_node->next_index = current_node->next_index;
