httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@attglobal.net>
Subject [PATCH] PR 8388, cgid, script not terminated when connection drops
Date Thu, 07 Nov 2002 21:00:34 GMT
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...

Mime
View raw message