You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@attglobal.net> on 2002/11/07 22:00:34 UTC
[PATCH] PR 8388, cgid, script not terminated when connection drops
when request is sent to daemon to spawn script, connection id is sent
too as a key for the script process... script process gets associated
with a pool which won't be cleaned up until handler tells daemon to
clean it up
when request_rec for CGI request is cleaned up, connection id is sent
over to cgi daemon again with a command that tells it to clean up the
special script pool... if the script is still running at that point,
it will be killed...
problems with this design:
. if script tries hard to stay alive unnecessarily (i.e., doesn't go
away until we send SIGKILL after an interval), then cgi daemon is
tied up doing that and is not handling new requests; it would be
better if the handler thread is tied up so that only that connection
is affected; but such code would be a little uglier since APR data
structures describing the script process can't be sent over the
socket back to the handler thread
probably this means that I need to dig out the native pid, ship it
back over the unix socket to the handler thread, and have the
handler thread do the dirty work
. this adds a new flow to the daemon per request for the cleanup
phase
Index: modules/generators/mod_cgid.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgid.c,v
retrieving revision 1.142
diff -u -r1.142 mod_cgid.c
--- modules/generators/mod_cgid.c 31 Oct 2002 09:30:43 -0000 1.142
+++ modules/generators/mod_cgid.c 7 Nov 2002 20:35:25 -0000
@@ -153,6 +153,7 @@
#define CGI_REQ 1
#define SSI_REQ 2
+#define CLN_REQ 3 /* clean up a pool associated with CGI_REQ or SSI_REQ */
/* DEFAULT_CGID_LISTENBACKLOG controls the max depth on the unix socket's
* pending connection queue. If a bunch of cgi requests arrive at about
@@ -266,7 +267,8 @@
}
#endif
-static int get_req(int fd, request_rec *r, char **argv0, char ***env, int *req_type)
+static int get_req(int fd, request_rec *r, char **argv0, char ***env, int *req_type,
+ unsigned long *conn_id)
{
int i, len, j, rc;
unsigned char *data;
@@ -281,6 +283,14 @@
if (rc != sizeof(int)) {
return 1;
}
+ rc = read(fd, conn_id, sizeof(*conn_id));
+ if (rc != sizeof(*conn_id)) {
+ return 1;
+ }
+ if (*req_type == CLN_REQ) {
+ return 0;
+ }
+
rc = read(fd, &j, sizeof(int));
if (rc != sizeof(int)) {
return 1;
@@ -410,11 +420,21 @@
"write to cgi daemon process");
}
+ /* Write the connection id. The daemon will use this
+ * as a hash value to find the script pid when it is
+ * time for that process to be cleaned up.
+ */
+
+ if (write(fd, &r->connection->id, sizeof(r->connection->id)) < 0) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, errno, r,
+ "write to cgi daemon process");
+ }
+
/* Write the number of entries in the environment. */
if (write(fd, &i, sizeof(int)) < 0) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, errno, r,
"write to cgi daemon process");
- }
+ }
for (i = 0; env[i]; i++) {
data = apr_pstrcat(r->pool, data, env[i], "\n", NULL);
@@ -511,6 +531,7 @@
server_rec *main_server = data;
cgid_server_conf *sconf = ap_get_module_config(main_server->module_config,
&cgid_module);
+ apr_hash_t *script_hash = apr_hash_make(pcgi);
apr_pool_create(&ptrans, pcgi);
@@ -574,6 +595,8 @@
apr_procattr_t *procattr = NULL;
apr_proc_t *procnew = NULL;
apr_file_t *inout;
+ unsigned long conn_id;
+ apr_pool_t *pscript;
apr_pool_clear(ptrans);
@@ -589,9 +612,8 @@
}
r = apr_pcalloc(ptrans, sizeof(request_rec));
- procnew = apr_pcalloc(ptrans, sizeof(*procnew));
r->pool = ptrans;
- rc = get_req(sd2, r, &argv0, &env, &req_type);
+ rc = get_req(sd2, r, &argv0, &env, &req_type, &conn_id);
if (rc) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0,
main_server,
@@ -599,6 +621,16 @@
close(sd2);
continue;
}
+
+ if (req_type == CLN_REQ) {
+ pscript = apr_hash_get(script_hash, &conn_id, sizeof(conn_id));
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, main_server,
+ "cleaning pool %pp for conn %lu", pscript, conn_id);
+ apr_pool_destroy(pscript);
+ close(sd2);
+ continue;
+ }
+
apr_os_file_put(&r->server->error_log, &errfileno, 0, r->pool);
apr_os_file_put(&inout, &sd2, 0, r->pool);
@@ -615,6 +647,12 @@
cmd_type = APR_PROGRAM;
}
+ /* data associated with the script process needs to live until
+ * request_rec back in Apache goes away... create a pool
+ * with such a lifetime
+ */
+ apr_pool_create(&pscript, pcgi);
+
if (((rc = apr_procattr_create(&procattr, ptrans)) != APR_SUCCESS) ||
((req_type == CGI_REQ) &&
(((rc = apr_procattr_io_set(procattr,
@@ -647,6 +685,7 @@
*/
close(sd2);
+ procnew = apr_pcalloc(pscript, sizeof(*procnew));
rc = ap_os_create_privileged_process(r, procnew, argv0, argv,
(const char * const *)env,
procattr, ptrans);
@@ -657,22 +696,13 @@
"couldn't create child process: %d: %s", rc,
apr_filename_of_pathname(r->filename));
}
+ else {
+ apr_pool_note_subprocess(pscript, procnew, APR_KILL_AFTER_TIMEOUT);
+ apr_hash_set(script_hash, &conn_id, sizeof(conn_id), pscript);
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, main_server,
+ "remembering pool %pp for conn %lu", pscript, conn_id);
+ }
}
- /* SNAFU: no call to apr_pool_note_subprocess() to cause the
- * CGI to be cleaned up when the request ends (in case
- * client drops connection)... can't write the pid back
- * to cgid_handler() because by the time we know the pid
- * the new CGI is already started and potentially writing
- * headers and body back to the handler...
- * perhaps cgid_handler() needs to provide a key with
- * a request to which cgid daemon will associate the
- * CGI pid... at request_rec cleanup time, that key
- * gets sent back to cgid_handler() telling it to kill
- * whatever pid is associated with the key...
- * 2 flows per request, which isn't cool... but
- * having the CGI running after client killed connection
- * isn't cool either...
- */
}
return -1;
}
@@ -1029,6 +1059,44 @@
*
* Actual cgid handling...
*/
+
+struct cleanup_script_info {
+ request_rec *r;
+ unsigned long conn_id;
+ cgid_server_conf *conf;
+};
+
+static apr_status_t tell_daemon_to_cleanup_script(void *vptr)
+{
+ struct cleanup_script_info *info = vptr;
+ int sd;
+ int rc;
+ int req_type = CLN_REQ;
+
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, info->r,
+ "need to tell daemon to clean up connid %lu",
+ info->conn_id);
+ rc = connect_to_daemon(&sd, info->r, info->conf);
+ if (rc != OK) {
+ return APR_EGENERAL;
+ }
+ if (write(sd, &req_type, sizeof(req_type)) != sizeof(req_type)) {
+ close(sd);
+ return APR_EGENERAL;
+ }
+ if (write(sd, &info->conn_id, sizeof(info->conn_id)) != sizeof(info->conn_id)) {
+ close(sd);
+ return APR_EGENERAL;
+ }
+ /* wait for connection to drop */
+ if (read(sd, &req_type, sizeof(req_type)) != 0) {
+ close(sd);
+ return APR_EGENERAL;
+ }
+ close(sd);
+ return APR_SUCCESS;
+}
+
static int cgid_handler(request_rec *r)
{
conn_rec *c = r->connection;
@@ -1042,6 +1110,7 @@
int sd;
char **env;
apr_file_t *tempsock;
+ struct cleanup_script_info *info;
if (strcmp(r->handler,CGI_MAGIC_TYPE) && strcmp(r->handler,"cgi-script"))
return DECLINED;
@@ -1111,6 +1180,13 @@
send_req(sd, r, argv0, env, CGI_REQ);
+ info = apr_palloc(r->pool, sizeof(struct cleanup_script_info));
+ info->r = r;
+ info->conn_id = r->connection->id;
+ info->conf = conf;
+ apr_pool_cleanup_register(r->pool, info,
+ tell_daemon_to_cleanup_script,
+ apr_pool_cleanup_null);
/* We are putting the socket discriptor into an apr_file_t so that we can
* use a pipe bucket to send the data to the client.
* Note that this does not register a cleanup for the socket. We did
--
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...