On Thu, Oct 28, 2010 at 8:57 AM, Jeff Trawick <traw...@gmail.com> wrote:
> On Wed, Oct 27, 2010 at 6:30 PM, Edward Z. Yang <ezy...@mit.edu> wrote:
>> Hello Jeff,
>>
>> We tested the latest version of mod_fcgid in trunk (r1028094) and
>> found that, while the total number of fcgis did not explode and overwhelm
>> the server, that after a ramp up time new FCGIs were not being spawned
>> and we were getting responses like:
>>
>> $ curl http://andersk.scripts.mit.edu/hello/hello.c.fcgi
>> curl: (52) Empty reply from server
>>
>> Our hypothesis is that the new code interacts poorly with mod_vhost_ldap
>> because mod_vhost_ldap copies the server_rec to a dynamically allocated
>> buffer
>> (see
>> http://mail-archives.apache.org/mod_mbox/httpd-dev/201007.mbox/%3c4c3465d9.1090...@rowe-clan.net%3e
>> so the current_node->server == procnode->server check never succeeds.
>> mod_ftp
>> does something similar.
>
> Interesting... I guess mod_fcgid has to leave some droplet attached
> to a server_rec that can used to identify the different <VirtualHost >
> containers in lieu of comparing the ServerName setting (original
> implementation) or the server_rec address (my subsequent "fix").
>
> (I don't know precisely what is failing in your case.)
>
> I guess you could rip out the server_rec comparison, as long as you
> have no VirtualHost-specific settings related to the FastCGI
> applications (i.e., if you can share your apps across vhosts).
>
> When somebody gets time/gumption I'm sure there will be a way to share
> apps across vhosts but that seems ugly to sanity check the feasibility
> (i.e., refuse to allow vhost-specific configuration of anything that
> would be impacted by reusing another vhost's process when there's some
> share-across-vhost flag at global scope. Hopefully your case can be
> made to work without that other work.
please try the attached patch if at all possible
when comparing vhosts it requires only that the per-server module
config be left alone in the server_rec copy
Index: modules/fcgid/fcgid_conf.c
===================================================================
--- modules/fcgid/fcgid_conf.c (revision 1027924)
+++ modules/fcgid/fcgid_conf.c (working copy)
@@ -57,10 +57,32 @@
#define DEFAULT_WRAPPER_KEY "ALL"
#define WRAPPER_FLAG_VIRTUAL "virtual"
+int fcgid_same_vhost(const server_rec *s1, const server_rec *s2)
+{
+ fcgid_server_conf *s1cfg, *s2cfg;
+
+ if (s1 == s2) {
+ return 1;
+ }
+
+ s1cfg = ap_get_module_config(s1->module_config, &fcgid_module);
+ s2cfg = ap_get_module_config(s2->module_config, &fcgid_module);
+
+ return s1cfg->server_id == s2cfg->server_id;
+}
+
void *create_fcgid_server_config(apr_pool_t * p, server_rec * s)
{
fcgid_server_conf *config = apr_pcalloc(p, sizeof(*config));
+ static int server_id = 0;
+ /* allow vhost comparison even when some mass-vhost module
+ * makes a copy of the server_rec to override docroot or
+ * other such settings
+ */
+ ++server_id;
+ config->server_id = server_id;
+
if (!s->is_virtual) {
config->busy_scan_interval = DEFAULT_BUSY_SCAN_INTERVAL;
config->error_scan_interval = DEFAULT_ERROR_SCAN_INTERVAL;
Index: modules/fcgid/fcgid_spawn_ctl.c
===================================================================
--- modules/fcgid/fcgid_spawn_ctl.c (revision 1027924)
+++ modules/fcgid/fcgid_spawn_ctl.c (working copy)
@@ -60,9 +60,9 @@
if (current_node->inode == procnode->inode
&& current_node->deviceid == procnode->deviceid
&& !strcmp(current_node->cmdline, procnode->cmdline)
- && current_node->server == procnode->server
&& current_node->uid == procnode->uid
- && current_node->gid == procnode->gid)
+ && current_node->gid == procnode->gid
+ && fcgid_same_vhost(current_node->server, procnode->server))
break;
previous_node = current_node;
}
@@ -173,9 +173,9 @@
if (current_node->inode == command->inode
&& current_node->deviceid == command->deviceid
&& !strcmp(current_node->cmdline, command->cmdline)
- && current_node->server == command->server
&& current_node->uid == command->uid
- && current_node->gid == command->gid)
+ && current_node->gid == command->gid
+ && fcgid_same_vhost(current_node->server, command->server))
break;
}
@@ -233,9 +233,9 @@
if (current_node->inode == procnode->inode
&& current_node->deviceid == procnode->deviceid
&& !strcmp(current_node->cmdline, procnode->cmdline)
- && current_node->server == procnode->server
&& current_node->uid == procnode->uid
- && current_node->gid == procnode->gid)
+ && current_node->gid == procnode->gid
+ && fcgid_same_vhost(current_node->server, procnode->server))
break;
}
Index: modules/fcgid/fcgid_conf.h
===================================================================
--- modules/fcgid/fcgid_conf.h (revision 1027924)
+++ modules/fcgid/fcgid_conf.h (working copy)
@@ -55,6 +55,8 @@
} fcgid_cmd_conf;
typedef struct {
+ /* not based on config */
+ int server_id;
/* global only */
apr_hash_t *cmdopts_hash;
int busy_scan_interval;
@@ -145,6 +147,7 @@
fcgid_cmd_env *cmdenv;
} fcgid_cmd_options;
+int fcgid_same_vhost(const server_rec *s1, const server_rec *s2);
void *create_fcgid_server_config(apr_pool_t * p, server_rec * s);
void *merge_fcgid_server_config(apr_pool_t * p, void *basev,
void *overridesv);
Index: modules/fcgid/fcgid_bridge.c
===================================================================
--- modules/fcgid/fcgid_bridge.c (revision 1027924)
+++ modules/fcgid/fcgid_bridge.c (working copy)
@@ -59,8 +59,8 @@
if (current_node->inode == inode
&& current_node->deviceid == deviceid
&& !strcmp(current_node->cmdline, cmdline)
- && current_node->server == server
- && current_node->uid == uid && current_node->gid == gid) {
+ && current_node->uid == uid && current_node->gid == gid
+ && fcgid_same_vhost(current_node->server, server)) {
/* Unlink from idle list */
previous_node->next_index = current_node->next_index;
@@ -139,9 +139,9 @@
if (current_node->inode == command->inode
&& current_node->deviceid == command->deviceid
&& !strcmp(current_node->cmdline, command->cmdline)
- && current_node->server == command->server
&& current_node->uid == command->uid
- && current_node->gid == command->gid) {
+ && current_node->gid == command->gid
+ && fcgid_same_vhost(current_node->server, command->server)) {
result++;
}
next_node = &proc_table[current_node->next_index];
