2.0 fixes no SIGSEGV if out of SysV sems, clean up SysV sems

[Jeff Trawick <tr...@ibm.net>]

I've tested this on FreeBSD 3.4 (uses SysV sems) and Linux (uses 
fcntl()).  prefork MPM on both; FreeBSD configuration hacked to avoid
-pthread, -D_REENTRANT, -lpthread, APR_HAS_THREADS, and anything
else which results in use of FreeBSD's userspace thread package
(free beer to whoever commits a change to APR configuration to
not go thread-wild when it finds pthread.h on systems with no
kernel threads (or who can convince me that this would be a bad idea))

[I'll be the first to admit that I'm just now learning Apache the
software and, perhaps more importantly, the Apache project way of
doing things.  I'm here to learn and appreciate your comments.]

Some of the code changed applies to lock mechanisms other than
semaphores, but the changes are in the context of some bad stuff
which can happen with semaphores.

1) don't SIGSEGV if out of SysV sems

(This will catch other errors as well, but the error scenario
I've tested is out-of-SysV-sems.)

This is a follow-up to Ryan's change which returns an error from
ap_init_alloc() when ap_create_mutex() fails.  Note that it is
not straightforward for the administrator to interpret the log
message since it could be any number of things which failed (but 
that conversation is over :) ).

I'm not real keen on exiting out like this, but until someone wants
to implement error handling in general for this early initialization
phase, it seems good enough.

--- http_main.c.old	Sun Feb 27 22:13:47 2000
+++ http_main.c	Tue Feb 29 14:30:56 2000
@@ -211,8 +211,16 @@
     
     {
 	ap_context_t *cntx;
+        ap_status_t stat;
+
+	stat = ap_create_context(&cntx, NULL);
+        if (stat != APR_SUCCESS) {
+            ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, NULL,
+                         "ap_create_context() failed to create "
+                         "initial context");
+            exit(1);
+        }
 
-	ap_create_context(&cntx, NULL);
 	process = ap_palloc(cntx, sizeof(process_rec));
 	process->pool = cntx;
     }

2) fix SysV sem version of lock_cleanup()

src/lib/apr/locks/unix/crossproc.c

The original logic is too similar to the fcntl() logic.  No real
cleanup is needed for fcntl() unless the lock is held because an 
unlink() is done on the file after it is created.  But for the SysV 
sems, we need to destroy them here.  Otherwise, we leave the stranded.
The comparison of interproc with -1 makes it work right if
lock_cleanup() is called after semget() fails.

--- crossproc.c.old	Tue Jan 25 18:04:27 2000
+++ crossproc.c	Tue Feb 29 14:33:04 2000
@@ -74,7 +74,7 @@
     struct lock_t *lock=lock_;
     union semun ick;
     
-    if (lock->curr_locked == 1) {
+    if (lock->interproc != -1) {
         ick.val = 0;
         semctl(lock->interproc, 0, IPC_RMID, ick);
     }

3) clean up alloc_mutex and spawn_mutex

src/lib/apr/lib/apr_pools.c

This too is a follow-on to Ryan's change to catch the error
from ap_create_lock() when getting spawn_mutex or alloc_mutex.

These guys don't get cleaned up automatically because at the 
time that cleanup is registered during creation of the locks, 
there is no context available, so they don't get chained into 
the context->pool->cleanups list.

There are two general cases to cover:

failed initialization of permanent_pool

- in this case, we don't expect the app (e.g., Apache) to call
  ap_destroy_pool() since it wasn't created in the first place,
  so in ap_init_alloc() make sure we don't leave a stranded lock

normal termination (after successful initialization of permanent_pool)

- we expect ap_destroy_pool() to get called for permanent_pool
  just before app termination (as Apache does in 
  destroy_and_exit_process()); look for that in ap_destroy_pool()
  so we know when to clean up the locks;

--- apr_pools.c.old	Mon Feb 28 14:14:35 2000
+++ apr_pools.c	Tue Feb 29 14:31:24 2000
@@ -570,6 +570,10 @@
     status2 = ap_create_lock(&spawn_mutex, APR_MUTEX, APR_INTRAPROCESS,
                    NULL, NULL);
     if (status1 != APR_SUCCESS || status2 != APR_SUCCESS) {
+        if (status1 == APR_SUCCESS) {
+	    ap_destroy_lock(alloc_mutex);
+            alloc_mutex = NULL;
+        }
         return NULL;
     }
 
@@ -581,6 +585,16 @@
     return permanent_pool;
 }
 
+static void term_alloc(void)
+{
+    if (alloc_mutex) {
+        ap_destroy_lock(alloc_mutex);
+    }
+    if (spawn_mutex) {
+        ap_destroy_lock(spawn_mutex);
+    }
+}
+
 void ap_destroy_real_pool(ap_pool_t *);
 
 /* We only want to lock the mutex if we are being called from ap_clear_pool.
@@ -657,6 +671,9 @@
 API_EXPORT(void) ap_destroy_pool(struct context_t *a)
 {
     ap_destroy_real_pool(a->pool);
+    if (a->pool == permanent_pool) {
+        term_alloc();
+    }
 }
 
 API_EXPORT(long) ap_bytes_in_pool(ap_pool_t *p)

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