httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <trawi...@bellsouth.net>
Subject [PATCH] alloc_mutex, spawn_mutex cleaned up too soon
Date Sun, 26 Mar 2000 19:37:27 GMT

notes:

. currently there is a problem in CVS (not to imply just one) which
  results in mmap failures on Linux; that error happens without
  this code !  here is the relevant log entry:

  [Sun Mar 26 14:33:08 2000] [crit] [client 127.0.0.1] default_handle
  r: mmap failed: /tmp/apache-2.0/htdocs/index.html

. see recent messages on mailing list with subject "alloc_mutex, 
  spawn_mutex cleaned up too soon" and to a lesser extent
  "Graceful restarts in Apache 2.0?" for background

. this fixes a problem where APR alloc mutexes were cleaned up
  prior to the last use of them; some systems don't like that; 
  Linux doesn't seem to give a ^%$$#;

. this fixes a problem on Win32 where ap_init_alloc()
  was called twice (both real-main() and Win32-main() called
  ap_create_context(&xyz, NULL), which is what used to trigger
  ap_init_alloc(); this wasn't reported, probably because it did
  hurt anything on systems currently being tested

. most testing on Linux with mpmt_pthread

. tested on Win98 minimally; winnt MPM is not Win9x-friendly, so
  it is hard to get very far; I would have preferred to play more on
  Win32 since this is where Allan found the problem, but no NT here

. I noticed that test/test_find.c and test/test_parser.c could stand 
  some updating to call ap_initialize()/ap_terminate() (not part of 
  this set of changes); I'll look at making those usable as a follow-on, 
  assuming that there aren't a hundred other things in those test drivers
  that need to be changed to make them work in the new world order

Index: CHANGES
===================================================================
RCS file: /cvs/apache/apache-2.0/src/CHANGES,v
retrieving revision 1.46
diff -u -r1.46 CHANGES
--- CHANGES	2000/03/25 15:00:08	1.46
+++ CHANGES	2000/03/26 17:45:13
@@ -1,4 +1,9 @@
 Changes with Apache 2.0a2-dev
+  *) Tweaked APR initialization and termination so that the
+     lifetime of memory management mutexes is longer than the
+     lifetime of managed memory.  APR apps must now call
+     ap_terminate(). [Jeff Trawick]
+
   *) Enabled layered I/O.  Docs can be found in the developers
      section of the manual.
      [Ryan Bloom]
Index: lib/apr/aprlib.def
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/aprlib.def,v
retrieving revision 1.12
diff -u -r1.12 aprlib.def
--- aprlib.def	2000/03/06 18:12:06	1.12
+++ aprlib.def	2000/03/26 17:45:14
@@ -236,4 +236,5 @@
 	ap_set_remote_port @215
       ap_open_stderr @216
         ap_set_pipe_timeout   @217
+        ap_terminate @218
 
Index: lib/apr/include/apr_general.h
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/include/apr_general.h,v
retrieving revision 1.18
diff -u -r1.18 apr_general.h
--- apr_general.h	2000/03/14 17:47:36	1.18
+++ apr_general.h	2000/03/26 17:45:14
@@ -228,6 +228,7 @@
                             ap_context_t *cont);
 ap_status_t ap_get_userdata(void **, char *key, ap_context_t *cont);
 ap_status_t ap_initialize(void);
+void        ap_terminate(void);
 ap_status_t ap_set_abort(int (*apr_abort)(int retcode), ap_context_t *cont);
 
 #ifdef __cplusplus
Index: lib/apr/include/apr_pools.h
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/include/apr_pools.h,v
retrieving revision 1.9
diff -u -u -r1.9 apr_pools.h
--- apr_pools.h 2000/03/10 00:06:09     1.9
+++ apr_pools.h 2000/03/26 18:09:46
@@ -135,7 +135,8 @@
  * currently being used...
  */
 
-ap_pool_t *ap_init_alloc(void);                /* Set up everything */
+ap_status_t ap_init_alloc(void);       /* Set up everything */
+void        ap_term_alloc(void);        /* Tear down everything */
 
 /* used to guarantee to the pool debugging code that the sub pool will not be
  * destroyed before the parent pool  
Index: lib/apr/lib/apr_pools.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/lib/apr_pools.c,v
retrieving revision 1.35
diff -u -r1.35 apr_pools.c
--- apr_pools.c	2000/03/21 03:35:34	1.35
+++ apr_pools.c	2000/03/26 17:45:16
@@ -115,7 +115,7 @@
  * containing lots of 0xa5s then you know something used data that's been
  * freed or uninitialized.
  */
-/* #define ALLOC_DEBUG */
+#define ALLOC_DEBUG                    <------- not to be committed :)
 
 /*
  * Debugging support: If defined all allocations will be done with
@@ -528,6 +528,9 @@
 	}
 	p->sub_pools = new_pool;
     }
+    else {
+        permanent_pool = p;
+    }
 
 #if APR_HAS_THREADS
     ap_unlock(alloc_mutex);
@@ -580,6 +583,7 @@
 
 static void * ap_pool_palloc(ap_pool_t *a, int reqsize, int (*apr_abort)(int retcode));
 
+#if 0
 static void ap_register_pool_cleanup(struct ap_pool_t *p, void *data,
 				      ap_status_t (*plain_cleanup) (void *),
 				      ap_status_t (*child_cleanup) (void *))
@@ -595,6 +599,7 @@
         p->cleanups = c;
     }
 }
+#endif
 
 API_EXPORT(void) ap_register_cleanup(struct context_t *p, void *data,
 				      ap_status_t (*plain_cleanup) (void *),
@@ -691,14 +696,8 @@
     /* do nothing cleanup routine */
     return APR_SUCCESS;
 }
-
-static ap_status_t cleanup_locks(void *data)
-{
-    ap_lock_t *thelock = (ap_lock_t *)data;
-    return ap_destroy_lock(thelock);
-}
 
-ap_pool_t *ap_init_alloc(void)
+ap_status_t ap_init_alloc(void)
 {
     ap_status_t status;
 #ifdef POOL_DEBUG
@@ -712,32 +711,29 @@
                    NULL, NULL);
     if (status != APR_SUCCESS) {
         ap_destroy_lock(alloc_mutex); 
-        return NULL;
+        return status;
     }
     status = ap_create_lock(&spawn_mutex, APR_MUTEX, APR_INTRAPROCESS,
                    NULL, NULL);
     if (status != APR_SUCCESS) {
         ap_destroy_lock(spawn_mutex); 
-        return NULL;
+        return status;
     }
 #endif
 
-    permanent_pool = ap_make_sub_pool(NULL, NULL);
-
-#if APR_HAS_THREADS
-    ap_register_pool_cleanup(permanent_pool, (void *)alloc_mutex, 
-                             cleanup_locks, 
-                             ap_null_cleanup);
-    ap_register_pool_cleanup(permanent_pool, spawn_mutex, 
-                             cleanup_locks, 
-                             ap_null_cleanup);
-#endif
-
 #ifdef ALLOC_STATS
     atexit(dump_stats);
 #endif
+
+    return APR_SUCCESS;
+}
 
-    return permanent_pool;
+void ap_term_alloc(void)
+{
+#if APR_HAS_THREADS
+    ap_destroy_lock(alloc_mutex);
+    ap_destroy_lock(spawn_mutex);
+#endif
 }
 
 void ap_destroy_real_pool(ap_pool_t *);
Index: lib/apr/misc/beos/start.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/misc/beos/start.c,v
retrieving revision 1.11
diff -u -r1.11 start.c
--- start.c	2000/03/22 14:09:03	1.11
+++ start.c	2000/03/26 17:45:16
@@ -64,7 +64,7 @@
         pool = ap_make_sub_pool(cont->pool, cont->apr_abort);
     }
     else {
-        pool = ap_init_alloc();;
+        pool = ap_make_sub_pool(NULL, NULL);
     }
         
     if (pool == NULL) {
@@ -142,6 +142,13 @@
 
 ap_status_t ap_initialize(void)
 {
-    return APR_SUCCESS;
+    ap_status_t status;
+    status = ap_init_alloc();
+    return status;
 }
- 
+
+void ap_terminate(void)
+{
+    ap_term_alloc();
+}
+
Index: lib/apr/misc/unix/start.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/misc/unix/start.c,v
retrieving revision 1.20
diff -u -r1.20 start.c
--- start.c	2000/03/14 17:47:38	1.20
+++ start.c	2000/03/26 17:45:16
@@ -74,7 +74,7 @@
         pool = ap_make_sub_pool(cont->pool, cont->apr_abort);
     }
     else {
-        pool = ap_init_alloc();;
+        pool = ap_make_sub_pool(NULL, NULL);
     }
         
     if (pool == NULL) {
@@ -188,8 +188,22 @@
  */
 ap_status_t ap_initialize(void)
 {
+    ap_status_t status;
     setup_lock();
-    return APR_SUCCESS;
+    status = ap_init_alloc();
+    return status;
+}
+
+/* ***APRDOC*******************************************************
+ * void ap_terminate(void)
+ *    Tear down any APR internal data structures which aren't
+ *    torn down automatically.  An APR program must call this
+ *    function at termination once it has stopped using APR
+ *    services.
+ */
+void ap_terminate(void)
+{
+    ap_term_alloc();
 }
 
 /* ***APRDOC********************************************************
Index: lib/apr/misc/win32/start.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/misc/win32/start.c,v
retrieving revision 1.13
diff -u -r1.13 start.c
--- start.c	2000/03/14 17:19:04	1.13
+++ start.c	2000/03/26 17:45:17
@@ -81,7 +81,7 @@
         pool = ap_make_sub_pool(cont->pool, cont->apr_abort);
     }
     else {
-        pool = ap_init_alloc();;
+        pool = ap_make_sub_pool(NULL, NULL);
     }
         
     if (pool == NULL) {
@@ -200,6 +200,7 @@
 /* This puts one thread in a Listen for signals mode */
 ap_status_t ap_initialize(void)
 {
+    ap_status_t status;
 #if 0
     unsigned tid;
 
@@ -211,5 +212,11 @@
         sleep(1);
     }
 #endif
-    return APR_SUCCESS;
+    status = ap_init_alloc();
+    return status;
+}
+
+void ap_terminate(void)
+{
+    ap_term_alloc();
 }
Index: main/http_main.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/main/http_main.c,v
retrieving revision 1.36
diff -u -r1.36 http_main.c
--- http_main.c	2000/03/23 14:48:45	1.36
+++ http_main.c	2000/03/26 17:45:18
@@ -193,6 +193,7 @@
 static void destroy_and_exit_process(process_rec *process, int process_exit_value)
 {
     ap_destroy_pool(process->pool); /* and destroy all descendent pools */
+    ap_terminate();
     exit(process_exit_value);
 }
 
@@ -209,6 +210,7 @@
             ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, NULL,
                          "ap_create_context() failed to create "
                          "initial context");
+            ap_terminate();
             exit(1);
         }
 
@@ -298,7 +300,9 @@
     ap_context_t *pcommands; /* Pool for -C and -c switches */
     module **mod;
 
+#ifndef WIN32 /* done in main_win32.c */
     ap_initialize();
+#endif
     process = create_process(argc, (const char **)argv);
     pglobal = process->pool;
     pconf = process->pconf;
Index: os/win32/main_win32.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/os/win32/main_win32.c,v
retrieving revision 1.9
diff -u -r1.9 main_win32.c
--- main_win32.c	2000/03/15 16:33:03	1.9
+++ main_win32.c	2000/03/26 17:45:21
@@ -155,6 +155,11 @@
     return;
     
 }
+static void cleanup_and_exit(void)
+{
+    ap_terminate();
+    exit(0);
+}
 int main(int argc, char *argv[])
 {
     ap_context_t *pwincmd;
@@ -171,18 +176,28 @@
     BOOLEAN install = FALSE;
     BOOLEAN uninstall = FALSE;
 
+    ap_status_t status;
     ap_array_header_t *cmdtbl;
     int new_argc = 0;
+
+    status = ap_initialize();
+    if (status != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_ERR, status, NULL, 
+                     "ap_initialize() failure");        
+        exit(0); /* nothing to clean up */
+    }
 
-    ap_create_context(&pwincmd, NULL);
-    if (pwincmd == NULL) {
-        exit(0);
+    status = ap_create_context(&pwincmd, NULL);
+    if (status != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_ERR, status, NULL, 
+                     "ap_create_context() failure");        
+        cleanup_and_exit();
     }
 
     /* Set up default server_root */
     if (!GetCurrentDirectory(sizeof(cwd),cwd)) {
         ap_log_error(APLOG_MARK,APLOG_ERR, GetLastError(), NULL, "GetCurrentDirectory() failure");
-        exit(0);
+        cleanup_and_exit();
     }
     server_root = ap_os_canonical_filename(pwincmd, cwd);
 
@@ -199,7 +214,7 @@
      */
     if (isProcessService()) {
         service_main(apache_main, argc, argv);
-        exit(0);
+        cleanup_and_exit();
     }
 
     /* Munch away at the command line arguments... */
@@ -282,20 +297,20 @@
         if (!service_name)
             service_name = DEFAULTSERVICENAME;
         InstallService(service_name, server_confname);
-        exit(0);
+        cleanup_and_exit();
     }
     /* Handle -u (uninstall service) */
     if (uninstall) {
         if (!service_name)
             service_name = DEFAULTSERVICENAME;
         RemoveService(service_name);
-        exit(0);
+        cleanup_and_exit();
     }
 
     /* Handle -k <signal> -n <service_name> */
     if (service_name && signal) {
         send_signal_to_service(service_name, signal);
-        exit(0);
+        cleanup_and_exit();
     }
 
     /* Handle -k restart|shutdown */
@@ -303,13 +318,13 @@
         char *s;
         s = read_config_cmd(server_confname, "pidfile", pwincmd);
         if (s == NULL) {
-            exit(0);
+            cleanup_and_exit();
         }
         if (!ap_os_is_path_absolute(s)) {
             s = ap_make_full_path(pwincmd, server_root, s);
         }
         signal_parent(s, signal, pwincmd);
-        exit(0);
+        cleanup_and_exit();
     }
 
     /* Complete building the new argv list. Need to add:


-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Mime
View raw message