You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by br...@apache.org on 2002/02/16 22:57:17 UTC

cvs commit: httpd-2.0/modules/mappers mod_rewrite.c

brianp      02/02/16 13:57:17

  Modified:    modules/mappers mod_rewrite.c
  Log:
  Converted the rewrite mapper lock to the new lock API
  
  Revision  Changes    Path
  1.96      +9 -7      httpd-2.0/modules/mappers/mod_rewrite.c
  
  Index: mod_rewrite.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_rewrite.c,v
  retrieving revision 1.95
  retrieving revision 1.96
  diff -u -r1.95 -r1.96
  --- mod_rewrite.c	29 Jan 2002 12:30:52 -0000	1.95
  +++ mod_rewrite.c	16 Feb 2002 21:57:17 -0000	1.96
  @@ -93,6 +93,7 @@
   #include "apr_user.h"
   #include "apr_lib.h"
   #include "apr_signal.h"
  +#include "apr_proc_mutex.h"
   
   #define APR_WANT_STRFUNC
   #define APR_WANT_IOVEC
  @@ -186,7 +187,7 @@
   static int proxy_available;
   
   static const char *lockname;
  -static apr_lock_t *rewrite_mapr_lock_acquire = NULL;
  +static apr_proc_mutex_t *rewrite_mapr_lock_acquire = NULL;
   static apr_lock_t *rewrite_log_lock = NULL;
   
   /*
  @@ -977,7 +978,8 @@
   
       if (lockname != NULL && *(lockname) != '\0')
       {
  -        rv = apr_lock_child_init (&rewrite_mapr_lock_acquire, lockname, p);
  +        rv = apr_proc_mutex_child_init(&rewrite_mapr_lock_acquire,
  +                                       lockname, p);
           if (rv != APR_SUCCESS) {
               ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
                            "mod_rewrite: could not init rewrite_mapr_lock_acquire "
  @@ -2929,7 +2931,7 @@
       /* take the lock */
   
       if (rewrite_mapr_lock_acquire) {
  -        apr_lock_acquire(rewrite_mapr_lock_acquire);
  +        apr_proc_mutex_lock(rewrite_mapr_lock_acquire);
       }
   
       /* write out the request key */
  @@ -2964,7 +2966,7 @@
   
       /* give the lock back */
       if (rewrite_mapr_lock_acquire) {
  -        apr_lock_release(rewrite_mapr_lock_acquire);
  +        apr_proc_mutex_unlock(rewrite_mapr_lock_acquire);
       }
   
       if (strcasecmp(buf, "NULL") == 0) {
  @@ -3266,8 +3268,8 @@
       lockname = ap_server_root_relative(p, lockname);
   
       /* create the lockfile */
  -    rc = apr_lock_create(&rewrite_mapr_lock_acquire, APR_MUTEX, APR_LOCKALL, 
  -                         APR_LOCK_DEFAULT, lockname, p);
  +    rc = apr_proc_mutex_create(&rewrite_mapr_lock_acquire, lockname,
  +                               APR_LOCK_DEFAULT, p);
       if (rc != APR_SUCCESS) {
           ap_log_error(APLOG_MARK, APLOG_ERR, rc, s,
                        "mod_rewrite: Parent could not create RewriteLock "
  @@ -3286,7 +3288,7 @@
       }
   
       /* destroy the rewritelock */
  -    apr_lock_destroy (rewrite_mapr_lock_acquire);
  +    apr_proc_mutex_destroy (rewrite_mapr_lock_acquire);
       rewrite_mapr_lock_acquire = NULL;
       lockname = NULL;
       return(0);
  
  
  

Re: cvs commit: httpd-2.0/modules/mappers mod_rewrite.c

Posted by Aaron Bannert <aa...@clove.org>.
On Sun, Feb 17, 2002 at 02:51:43PM -0800, Brian Pane wrote:
> You're right; the apr_proc_mutex_t won't be sufficient in this
> case.  We'll really need apr_global_mutex_t for this.  If that's
> going to be available soon, I'll switch the rewrite code to use
> it when it's released; but if not, I'll revert to the old lock
> API for now.

I'm working on it right now, and I may have it on unix in the next hour or
two.

-aaron

[PATCH] change mod_rewrite to use apr_global_mutex_t

Posted by Aaron Bannert <aa...@clove.org>.
On Sun, Feb 17, 2002 at 02:51:43PM -0800, Brian Pane wrote:
> You're right; the apr_proc_mutex_t won't be sufficient in this
> case.  We'll really need apr_global_mutex_t for this.  If that's
> going to be available soon, I'll switch the rewrite code to use
> it when it's released; but if not, I'll revert to the old lock
> API for now.
> --Brian

I've committed the new API for Unix, but will not make the conversion
to the new API until the other platforms are up to speed. In the
mean time, here is a patch to convert to the new API for mod_rewrite.
I've tested it as well as I can, but I could use some help exercising
the new lock code in a massively concurrent environment:

-aaron


Index: modules/mappers/mod_rewrite.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_rewrite.c,v
retrieving revision 1.96
diff -u -r1.96 mod_rewrite.c
--- modules/mappers/mod_rewrite.c	16 Feb 2002 21:57:17 -0000	1.96
+++ modules/mappers/mod_rewrite.c	18 Feb 2002 01:29:07 -0000
@@ -93,6 +93,7 @@
 #include "apr_user.h"
 #include "apr_lib.h"
 #include "apr_signal.h"
+#include "apr_global_mutex.h"
 #include "apr_proc_mutex.h"
 
 #define APR_WANT_STRFUNC
@@ -188,7 +189,7 @@
 
 static const char *lockname;
 static apr_proc_mutex_t *rewrite_mapr_lock_acquire = NULL;
-static apr_lock_t *rewrite_log_lock = NULL;
+static apr_global_mutex_t *rewrite_log_lock = NULL;
 
 /*
 ** +-------------------------------------------------------+
@@ -942,8 +943,8 @@
     proxy_available = (ap_find_linked_module("mod_proxy.c") != NULL);
 
     /* create the rewriting lockfiles in the parent */
-    if ((rv = apr_lock_create(&rewrite_log_lock, APR_MUTEX, APR_LOCKALL,
-                              APR_LOCK_DEFAULT, NULL, p)) != APR_SUCCESS) {
+    rv = apr_global_mutex_create(&rewrite_log_lock, NULL, APR_LOCK_DEFAULT, p);
+    if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
                      "mod_rewrite: could not create rewrite_log_lock");
         return HTTP_INTERNAL_SERVER_ERROR;
@@ -3218,10 +3219,10 @@
                 (unsigned long)(r->server), (unsigned long)r,
                 type, redir, level, str2);
 
-    apr_lock_acquire(rewrite_log_lock);
+    apr_global_mutex_lock(rewrite_log_lock);
     nbytes = strlen(str3);
     apr_file_write(conf->rewritelogfp, str3, &nbytes);
-    apr_lock_release(rewrite_log_lock);
+    apr_global_mutex_unlock(rewrite_log_lock);
 
     va_end(ap);
     return;

Re: cvs commit: httpd-2.0/modules/mappers mod_rewrite.c

Posted by Brian Pane <bp...@pacbell.net>.
You're right; the apr_proc_mutex_t won't be sufficient in this
case.  We'll really need apr_global_mutex_t for this.  If that's
going to be available soon, I'll switch the rewrite code to use
it when it's released; but if not, I'll revert to the old lock
API for now.
--Brian

Aaron Bannert wrote:

>On Sat, Feb 16, 2002 at 09:57:17PM -0000, brianp@apache.org wrote:
>
>>brianp      02/02/16 13:57:17
>>
>>  Modified:    modules/mappers mod_rewrite.c
>>  Log:
>>  Converted the rewrite mapper lock to the new lock API
>>
>
>Did mod_rewrite actually need a LOCKALL? If so, apr_proc_mutex_t
>is an insufficient replacement, since it can not guarantee mutual
>exclusion of threads in the same process.
>
>-aaron
>
>>  -    rc = apr_lock_create(&rewrite_mapr_lock_acquire, APR_MUTEX, APR_LOCKALL, 
>>  -                         APR_LOCK_DEFAULT, lockname, p);
>>  +    rc = apr_proc_mutex_create(&rewrite_mapr_lock_acquire, lockname,
>>  +                               APR_LOCK_DEFAULT, p);
>>
>




Re: cvs commit: httpd-2.0/modules/mappers mod_rewrite.c

Posted by Aaron Bannert <aa...@clove.org>.
On Sat, Feb 16, 2002 at 09:57:17PM -0000, brianp@apache.org wrote:
> brianp      02/02/16 13:57:17
> 
>   Modified:    modules/mappers mod_rewrite.c
>   Log:
>   Converted the rewrite mapper lock to the new lock API

Did mod_rewrite actually need a LOCKALL? If so, apr_proc_mutex_t
is an insufficient replacement, since it can not guarantee mutual
exclusion of threads in the same process.

-aaron

>   -    rc = apr_lock_create(&rewrite_mapr_lock_acquire, APR_MUTEX, APR_LOCKALL, 
>   -                         APR_LOCK_DEFAULT, lockname, p);
>   +    rc = apr_proc_mutex_create(&rewrite_mapr_lock_acquire, lockname,
>   +                               APR_LOCK_DEFAULT, p);