All,

Months ago, Madhu posted a patch to put thread-safe locks around the Post
logging in mod_specweb99... here's an alternative using apr_global_mutex
locks:

Index: mod_specweb99.c
===================================================================
RCS file: /home/cvs/httpd-test/specweb99/specweb99-2.0/mod_specweb99.c,v
retrieving revision 1.28
diff -u -r1.28 mod_specweb99.c
--- mod_specweb99.c     25 Jun 2003 18:51:11 -0000      1.28
+++ mod_specweb99.c     6 Aug 2003 01:29:29 -0000
@@ -84,6 +84,20 @@
 #include <unistd.h>
 #endif
 
+#if !defined(OS2) && !defined(WIN32) && !defined(BEOS)  &&
!defined(NETWARE)
+#include "unixd.h"
+#define MOD_SPECWEB_SET_MUTEX_PERMS /* XXX Apache should define something
*/
+#endif
+
+/* Hardcoded lock file path for the Post log file mutex. Whether this file
+ * will actually be created depends on the default global mutex
implementation
+ * that APR was compiled with.
+ *
+ * Global mutex structure pointer for Post log mutex.
+ */
+#define LOCKFILENAME "/tmp/specweb99_lockfile"
+apr_global_mutex_t *log_mutex;
+
 /* Note: version must be of the x.yy type - as it is
  * send over the http protocol wire; where x and y
  * are single 0-9 ascii digits :-). The name should
@@ -559,16 +573,57 @@
     return (void *) _my;
 }
 
+/* Get rid of the Post log mutex. This function is registered as
+ * cleanup for the pool that we are passed when the parent is
+ * initialized. 
+ */
+ 
+static apr_status_t log_mutex_remove(void *data)
+{
+    apr_global_mutex_destroy(log_mutex);
+    log_mutex = NULL;
+    return(0);
+}
+
 static int specweb99_module_init(apr_pool_t * p, apr_pool_t * plog,
                                  apr_pool_t * ptemp, server_rec *s)
 {
+    apr_status_t rv;
+    
     ap_add_version_component(p, NAME "/" VERSION);
 
     ap_log_error(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, s,
                  NAME "/" VERSION " module: Compiled on %s at %s",
__DATE__,
                  __TIME__);
+    
+    /* Create the Post log mutex, using the default global locking method
that 
+     * APR was compiled with.
+     */
+    rv = apr_global_mutex_create(&log_mutex, LOCKFILENAME,
APR_LOCK_DEFAULT, p);
+    if (rv != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+            "mod_specweb99: Parent could not create Post log mutex "
+            "with file %s", LOCKFILENAME);
+        return HTTP_INTERNAL_SERVER_ERROR;
+    }
+    
+    /* This is defined at the top of this file and causes the permissions
+     * function to not get called on platforms that don't require it.
+     */
+#ifdef MOD_SPECWEB_SET_MUTEX_PERMS
+    rv = unixd_set_global_mutex_perms(log_mutex);
+    if (rv != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+            "mod_specweb99: Parent could not set permissions "
+            "on Post log mutex; check User and Group directives");
+        return HTTP_INTERNAL_SERVER_ERROR;
+    }
+#endif /* MOD_SPECWEB_SET_MUTEX_PERMS */
+    
+    apr_pool_cleanup_register(p, (void *)s, log_mutex_remove,
+                              apr_pool_cleanup_null);
 
-    return 0;
+    return OK;
 }
 
 static void specweb99_child_init(apr_pool_t * p, server_rec *s)
@@ -615,6 +670,13 @@
          * concept of a docroot).
          */
     };
+
+    /* Re-open the Post log mutex for this child. I assume this does the
+     * initialization of the intra-process part of the global mutex.
+     */
+    if (apr_global_mutex_child_init(&log_mutex, LOCKFILENAME, p) !=
APR_SUCCESS)
+        exit(APEXIT_CHILDFATAL);
+
 }               /* specweb99_child_init */
 
 static int do_housekeeping(request_rec *r)
@@ -1160,7 +1222,7 @@
         return HTTP_INTERNAL_SERVER_ERROR;
     }
 
-    if ((rv = _wlock(r->server, r, f, _my->log_path)) != APR_SUCCESS) {
+    if ((rv = apr_global_mutex_lock(log_mutex)) != APR_SUCCESS) {
         returnHTMLPageWithMessage(r, "Failed to lock post.log file");
     }
     else {
@@ -1174,7 +1236,7 @@
         }
     }
 
-    if ((rv2 = apr_file_unlock(f)) != APR_SUCCESS) {
+    if ((rv2 = apr_global_mutex_unlock(log_mutex)) != APR_SUCCESS) {
         if (rv == APR_SUCCESS) {
             rv = rv2;
             ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

(also attached to get around line-wrapping by the mailer).

This approach is also used in mod_rewrite. I know apr_global_mutexes are
advertised as potentially expensive, but in a hybrid situation you're going
to end up doing something like this, and apr_global_mutexes turn into
process locks when APR is thread-free or (not tested) on Win32.

Tested on Darwin under light load and Solaris under moderate load. Thoughts,
opinions, comments? Or shall I just commit the thing?

Thanks,

S.

-- 
Covalent Technologies                             [EMAIL PROTECTED]
Engineering group                                Voice: (415) 856 4214
303 Second Street #375 South                       Fax: (415) 856 4210
San Francisco CA 94107

   PGP Fingerprint: 7A8D B189 E871 80CB 9521  9320 C11E 7B47 964F 31D9

=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited.  If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================

Attachment: mod_specweb99.c.patch
Description: Binary data

Reply via email to