You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@apr.apache.org by Christian Gross <Ch...@yahoo.de> on 2001/04/30 00:13:48 UTC

[PATCH] RW lock Iteration 2 for Windows

Hi

Here is the second iteration of the RW lock code for the Windows.  It
includes the fix for the cross-process RW lock.

Christian 

===================================================================
--- c:/projects/apr/include/apr_errno.h	Sun Apr  8 06:19:36 2001
+++ c:/apr/include/apr_errno.h	Sat Apr 28 17:22:56 2001
@@ -248,7 +248,7 @@
 #define APR_EINCOMPLETE    (APR_OS_START_ERROR + 22)
 #define APR_EABOVEROOT     (APR_OS_START_ERROR + 23)
 #define APR_EBADPATH       (APR_OS_START_ERROR + 24)
-
+#define APR_ELOCKTYPE      (APR_OS_START_ERROR + 25)
 
 /* APR ERROR VALUE TESTS */
 #define APR_STATUS_IS_ENOSTAT(s)        ((s) == APR_ENOSTAT)
--- c:/projects/apr/include/apr_lock.h	Thu Feb 15 23:41:18 2001
+++ c:/apr/include/apr_lock.h	Sun Apr 29 12:39:37 2001
@@ -70,6 +70,7 @@
 typedef enum {APR_CROSS_PROCESS, APR_INTRAPROCESS, APR_LOCKALL}
apr_lockscope_e;
 
 typedef enum {APR_MUTEX, APR_READWRITE} apr_locktype_e;
+typedef enum {APR_READER, APR_WRITER} apr_readerwriterlock_e;
 
 typedef struct apr_lock_t           apr_lock_t;
 
@@ -103,13 +104,20 @@
                                           apr_lockscope_e scope,
                                           const char *fname,
                                           apr_pool_t *cont);
-
 /**
  * Lock a protected region.
  * @param lock The lock to set.
- * @deffunc apr_status_t apr_lock_acquire(apr_lock_t *lock)
  */
 APR_DECLARE(apr_status_t) apr_lock_acquire(apr_lock_t *lock);
+
+/**
+ * Lock a region with either a reader or writer lock.
+ * @param lock The lock to set.
+ * @param type The type of lock to acquire.
+ * @deffunc apr_status_t apr_lock_acquire_rw(apr_lock_t *lock,
apr_readerwriterlock_e type)
+ */
+APR_DECLARE(apr_status_t) apr_lock_acquire_rw(apr_lock_t *lock,
+                                              apr_readerwriterlock_e
type);
 
 /**
  * Unlock a protected region.
--- c:/projects/apr/include/arch/win32/locks.h	Thu Feb 15 23:41:24
2001
+++ c:/apr/include/arch/win32/locks.h	Sun Apr 29 18:04:02 2001
@@ -57,6 +57,10 @@
 
 #include "apr_lock.h"
 
+#define DOING_NOTHING       0
+#define IS_READING			1
+#define IS_WRITING			2
+
 struct apr_lock_t {
     apr_pool_t *cntxt;
     apr_locktype_e type;
@@ -64,6 +68,14 @@
     HANDLE mutex;
     CRITICAL_SECTION section;
     char *fname;
+	/* Declarations used for the reader writer implementation */
+    apr_uint32_t activeReaders;
+    apr_uint32_t activeWriters;
+    apr_uint32_t waitingReaders;
+    apr_uint32_t waitingWriters;
+    HANDLE blockedReader;
+    HANDLE blockedWriter;
+    apr_uint32_t currOperation;
 };
 
 #endif  /* LOCKS_H */
--- c:/projects/apr/locks/win32/locks.c	Thu Feb 15 23:41:28 2001
+++ c:/apr/locks/win32/locks.c	Sun Apr 29 18:08:36 2001
@@ -75,6 +75,13 @@
     newlock->fname = apr_pstrdup(cont, fname);
     newlock->type = type;
     newlock->scope = scope;
+    newlock->type = type;
+    newlock->scope = scope;
+    newlock->activeReaders = 0;
+    newlock->activeWriters = 0;
+    newlock->waitingReaders = 0;
+    newlock->waitingWriters = 0;
+    newlock->currOperation = DOING_NOTHING;
     sec.nLength = sizeof(SECURITY_ATTRIBUTES);
     sec.lpSecurityDescriptor = NULL;
 
@@ -85,10 +92,26 @@
         sec.bInheritHandle = FALSE;
     }
 
+    if (newlock->type == APR_MUTEX) {
+        newlock->blockedReader = NULL;
+        newlock->blockedWriter = NULL;
+    }
     if (scope == APR_INTRAPROCESS) {
         InitializeCriticalSection(&newlock->section);
+        if (newlock->type == APR_READWRITE) {
+            newlock->blockedReader = CreateMutex(NULL, FALSE, NULL);
+	        newlock->blockedWriter = CreateMutex(NULL, FALSE,
NULL);
+        }    
     } else {
         newlock->mutex = CreateMutex(&sec, FALSE, fname);
+        if (newlock->type == APR_READWRITE) {
+            char *tmp;
+
+            tmp = apr_pstrcat( cont, fname, ".BlockedReader", NULL);
+            newlock->blockedReader = CreateMutex(&sec, FALSE, tmp);
+            tmp = apr_pstrcat( cont, fname, ".BlockedWriter", NULL);
+	        newlock->blockedWriter = CreateMutex(&sec, FALSE,
NULL);
+        }    
     }
     *lock = newlock;
     return APR_SUCCESS;
@@ -115,13 +138,20 @@
     return APR_SUCCESS;
 }
 
-APR_DECLARE(apr_status_t) apr_lock_acquire(apr_lock_t *lock)
+/*
+ * This is private routine to get the lock 
+ * It is made private because both the regular lock routines
+ * and the reader writer lock routines use it
+ */
+static apr_status_t get_lock(apr_lock_t * lock)
 {
     DWORD rv;
+
     if (lock->scope == APR_INTRAPROCESS) {
         EnterCriticalSection(&lock->section);
         return APR_SUCCESS;
-    } else {
+    }
+    else {
         rv = WaitForSingleObject(lock->mutex, INFINITE);
 
         if (rv == WAIT_OBJECT_0 || rv == WAIT_ABANDONED) {
@@ -131,12 +161,13 @@
     return apr_get_os_error();
 }
 
-APR_DECLARE(apr_status_t) apr_lock_release(apr_lock_t *lock)
+static apr_status_t release_lock(apr_lock_t * lock)
 {
     if (lock->scope == APR_INTRAPROCESS) {
         LeaveCriticalSection(&lock->section);
         return APR_SUCCESS;
-    } else {
+    }
+    else {
         if (ReleaseMutex(lock->mutex) == 0) {
             return apr_get_os_error();
         }
@@ -144,6 +175,107 @@
     return APR_SUCCESS;
 }
 
+APR_DECLARE(apr_status_t) apr_lock_acquire_rw(apr_lock_t *lock,
+                                              apr_readerwriterlock_e
type)
+{
+    DWORD rv;
+
+    if (lock->type == APR_MUTEX) {
+        return APR_ELOCKTYPE;
+    }
+
+    rv = get_lock(lock);
+    if (rv != APR_SUCCESS) {
+        return rv;
+    }
+    
+    if (type == APR_WRITER) {
+        if (lock->activeReaders == 0 && lock->activeWriters == 0) {
+		    /* there is no active reader or writer, OK to
start writing */
+            lock->activeWriters = 1;
+            lock->currOperation = IS_WRITING;
+            release_lock(lock);
+        }
+        else {
+            /* there is active readers or writer, hold on until free
*/
+            lock->waitingWriters++;
+            release_lock(lock);
+            WaitForSingleObject(lock->blockedWriter, INFINITE);
+        }
+    }
+    else if (type == APR_READER) {
+        if (lock->activeWriters > 0 || lock->waitingWriters > 0) {
+            lock->waitingReaders++;
+            release_lock(lock);
+            WaitForSingleObject(lock->blockedReader, INFINITE);
+        }
+        else {
+            lock->activeReaders++;
+            lock->currOperation = IS_WRITING;
+            release_lock(lock);
+        }
+    }
+    return APR_SUCCESS;
+}
+
+APR_DECLARE(apr_status_t) apr_lock_acquire(apr_lock_t * lock)
+{
+    /* ToDo: if the lock is a read write what is the default
behaviour?
+     *   Right now it is set to return an error
+     */
+    if (lock->type == APR_READWRITE) {
+        return APR_ELOCKTYPE;
+    }
+    return get_lock(lock);
+}
+
+APR_DECLARE(apr_status_t) apr_lock_release(apr_lock_t * lock)
+{
+    DWORD rv;
+
+    if (lock->type == APR_MUTEX) {
+        return release_lock(lock);
+    }
+    else {
+        rv = get_lock(lock);
+        if (rv != APR_SUCCESS) {
+            return rv;
+        }
+        if (lock->currOperation == IS_READING) {
+            lock->activeReaders--;
+            /* last reader thread to finish reading needs 
+            * to activate a waiting writer 
+            */
+            if (lock->activeReaders == 0 && lock->waitingWriters > 0)
{
+                lock->activeWriters = 1;
+                lock->waitingWriters--;
+                ReleaseMutex(lock->blockedWriter);
+            }
+        }
+        else if (lock->currOperation == IS_WRITING) {
+            lock->activeWriters = 0;
+            if (lock->waitingReaders > 0) {
+                /* if there are waiting readers, release them all
from read queue */
+                while (lock->waitingReaders > 0) {
+                    lock->waitingReaders--;
+                    lock->activeReaders++;
+                    ReleaseMutex(lock->blockedWriter);
+                }
+            }
+            else if (lock->waitingWriters > 0) {
+                /* no waiting reader and we have waiting writer, 
+                * release 1 writer from write queue
+                */
+                lock->waitingWriters--;
+                ReleaseMutex(lock->blockedWriter);
+            }
+        }
+        lock->currOperation = DOING_NOTHING;
+        release_lock(lock);
+    }
+    return APR_SUCCESS;
+}
+
 APR_DECLARE(apr_status_t) apr_lock_destroy(apr_lock_t *lock)
 {
     if (lock->scope == APR_INTRAPROCESS) {
@@ -153,6 +285,11 @@
         if (CloseHandle(lock->mutex) == 0) {
             return apr_get_os_error();
         }
+    }
+
+    if (lock->type == APR_READWRITE) {
+        CloseHandle(lock->blockedReader);
+        CloseHandle(lock->blockedWriter);
     }
     return APR_SUCCESS;
 }

Re: [PATCH] RW lock Iteration 2 for Windows

Posted by Christian Gross <Ch...@yahoo.de>.

On Mon, 30 Apr 2001 12:31:27 -0400, you wrote:
>
>This looks wrong.  An APR_INTRAPROCESS lock should use a critical section, not a mutex. 
>
I was wondering about this.  How come the use of a critical section?
The only advantage that critical section offers is speed.  But even
that is debatable.  If there is nothing holding the lock the program
will stay in user mode.  And then acquiring the lock is REALLY fast.
But if most of your time nobody is holding a lock, which the locking
code in the first place.  If something is holding the lock then the
program needs to do a switch to kernel mode.  This makes the critical
section just as slow as a mutex.  

In general I tend to use mutexs because they are more flexible, and
when you abandon them the locks held are released.  I tend to find
that with critical sections I will deadlock code more readily than
with mutexes.  For example the prime problem is getting a lock twice
within the same thread.  A mutex has no problem with this, but a
critical section will perform an automatic deadlock.

Any comments....

Christian Gross

Re: [PATCH] RW lock Iteration 2 for Windows

Posted by Justin Erenkrantz <je...@ebuilt.com>.

On Mon, Apr 30, 2001 at 03:11:29PM -0400, Christian Gross wrote:
> Right now it is this.
> 
> struct apr_lock_t {
>     apr_pool_t *cntxt;
>     apr_locktype_e type;
>     apr_lockscope_e scope;
>     HANDLE mutex;
>     CRITICAL_SECTION section;
>     char *fname;
> };
> 
> Would it not be better to do the following?
> 
> struct apr_lock_t {
>     apr_pool_t *cntxt;
>     apr_locktype_e type;
>     apr_lockscope_e scope;
>     union {
>         HANDLE mutex;
>         CRITICAL_SECTION section;
>     };
>     char *fname;

I thought about that as well for Unix, but I'm not sure what the policy
on unions are in Apache code.  I don't think I've seen them used (I
may very well be wrong).  You might get into issues with size of the
structures, but the compiler might handle that for you (is one structure
larger than another?).  I forget.  K&R *probably* specifies this...  
-- justin

Re: [PATCH] RW lock Iteration 2 for Windows

Posted by Christian Gross <Ch...@yahoo.de>.

On Mon, 30 Apr 2001 12:31:27 -0400, you wrote:
>This looks wrong.  An APR_INTRAPROCESS lock should use a critical section, not a mutex. 

Ooops...  

Now I remember why I also use Mutexes.  Mutexes can be signaled and
with the reader writer code I need to be able to signal a single
blocked writer or reader.  This is not possible with a critical
section.

BTW I was also wondering about the structure in locks.h.

Right now it is this.

struct apr_lock_t {
    apr_pool_t *cntxt;
    apr_locktype_e type;
    apr_lockscope_e scope;
    HANDLE mutex;
    CRITICAL_SECTION section;
    char *fname;
};

Would it not be better to do the following?

struct apr_lock_t {
    apr_pool_t *cntxt;
    apr_locktype_e type;
    apr_lockscope_e scope;
    union {
        HANDLE mutex;
        CRITICAL_SECTION section;
    };
    char *fname;
};


Christian

Re: [PATCH] RW lock Iteration 2 for Windows

Posted by Christian Gross <Ch...@yahoo.de>.

That is what I thought.

I can change the code, but to be honest I do prefer Mutexes...

The lock contention speed slow down is something that I seen Richter
talk about.

Christian

On Mon, 30 Apr 2001 13:05:02 -0400, you wrote:

>The peformance numbers I have seen indicate that a critical section is somewhat faster than a mutex,
>even when there is little or no lock contention. I've not done the analysis myself though.
>
>Bill
>

Re: Problems with APR under Linux...

Posted by rb...@covalent.net.

On Mon, 21 May 2001, jean-frederic clere wrote:

> Greg Stein wrote:
> >
> > On Sat, May 19, 2001 at 01:05:17AM +0100, Pier P. Fumagalli wrote:
> > > Pier P. Fumagalli at pier@betaversion.org wrote:
> > >
> > > > jean-frederic clere at jfrederic.clere@fujitsu-siemens.com wrote:
> > > >
> > > >> "Pier P. Fumagalli" wrote:
> > > >>>
> > > >>> I'm a dork, I sent it only to Bill without including the mailing list...
> > > >>> Damn lack of a Reply-To header :) :)
> > > >>>
> > > >>>     Pier
> > > >>>
> > > >>> [jreilly@lorien bin]$ ./httpd
> > > >>> Syntax error on line 957 of /usr/local/apache/conf/httpd.conf:
> > > >>> Cannot load /usr/local/apache/libexec/mod_webapp.so into server:
> > > >>> /usr/local/apache/libexec/mod_webapp.so: undefined symbol: pthread_sigmask
> > > >>> [jreilly@lorien bin]$
> > > >>>
> > > >>> That's weird... Do anyone knows where this one could come from? It seems
> > > >>> that pthread_sigmask is not found when talking about Linux, but that seems
> > > >>> really strange, since I don't use any threading function per se in WebApp...
> > > >>> (Might be called from apr_initialize?)
> > > >>
> > > >> I have no found exactly what happends but just add -lpthread in the
> > > >> (tomcat-connectors)/Makefile solves the problem:
> > > >
> > > > Ok, will add that into our autoconf/automake stuff. :) Thanks for the
> > > > hint...
> > > >
> > > >   Pier
> > >
> > > Jean-Federic  told me that the trick works for Linux, but not for other
> > > systems such as Reliant Unix... Is there a way to find out on which
> > > libraries APR relies on?
> >
> > Right. Adding -lpthread manually is absolutely the wrong thing for your app
> > to do. That was poor advice.
> >
> > APR generates a shell script called APRVARS. That should have everything
> > that you need for compiling your app, and for linking your app to APR and
> > its dependent libraries.
> >
> > Note: it is best to compile your app with the flags from APRVARS so that you
> > don't get skewed compile options between APR and your app. Yes, there are
> > well-defined binary interfaces, but heck: APR figured it all out for you, so
> > go ahead and use it :-)
>
> Just a question, is there a HOW about "How to use APR".

When I first wrote APR, I wrote a doc called APRDesign.  That doc is still
in the root of the tree, but it is a bit out of date.  It has the general
ideas, but it doesn't even come close to telling you everything.  :-(

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------