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/29 01:29:02 UTC
WriteLock ++ ;)
I just looked at the UNIX based RW lock stuff. There is one
difference between what I uploaded and what Justin uploaded. He used
one method call to acquire a reader or writer. I used two. Which one
is right?
I used two function calls because I thought it would keep things as
simple and fast as possible. I looked in the UNIX code and within the
apr_lock_acquire_rw routine there is a switch to test which type of
lock to acquire. I would tend to say two method calls would be faster
(OK it is splitting hairs ;) )
Another question that I have is follows.
A user creates a regular lock and calls the RW lock routines or
creates a RW lock and calls the regular lock routines. What happens?
In the Windows PATCH I decided to return an error saying this is not
possible. However because of that I had to modify the apr_errno.h
file to include an extra error code.
BTW I just caught an error in the Windows routines in that when the
lock is cross process my reader writer will fail. Need to fix that...
Christian
Re: WriteLock ++ ;)
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sat, Apr 28, 2001 at 09:28:05PM -0400, Christian Gross wrote:
> Anyone on the Apache APR team wish to make a comment? Justin, if you
> are please excuse my ignorance.
Nope. Otherwise, I probably would have committed the lock stuff by
now. =) Even then, most people on the APR "team" aren't that different
from any of us "not" on the team. The only thing they have on us is
"commit" access. Big deal. I can create my own CVS repository! =)
> On the Windows platform the locks can be cross-process. Hence right
> now there is the problem that the RW locks will break between
> processes. If they cannot be supported on the POSIX platform then
> return APR_ENOTIMPL probably would be a good thing.
Deal. That is attached. I also removed the call to apr_lock_acquire_rw
in apr_lock_acquire for APR_READWRITE. Makes more sense when I think
about it.
> >We should probably integrate our patches into one to make it easier
> >on others...
> >
> Ah, yes I agree with you. So how about we do the following. One
> function, but return an error when using the wrong lock acquire
> function is called?
>
> All OK? Any comments?
Okay. I looked over your patch and you do some reformats of the code
that isn't essential to the locking - you probably want to lose that.
I did some cleaning up to locks.c when I was working with it, then I
ditched my format changes before I submitted the patch. Can you
regenerate your patches without the formatting and use "cvs diff -u"
rather than a normal file-based diff? I can't do anything with all of
those forward-slashes. =)
To be precise, the formatting that you did doesn't fit the Apache-style
(like there *is* one - we can't even agree on tabs v. spaces!). NBD.
See http://dev.apache.org/styleguide.html for the "official" style.
> >Two platforms down, two to go. -- justin
> Yupe....
Cool. I don't know if the Bills can/will verify the patches. I have
no clue about Win32. -- justin
Index: configure.in
===================================================================
RCS file: /home/cvspublic/apr/configure.in,v
retrieving revision 1.294
diff -u -r1.294 configure.in
--- configure.in 2001/04/27 18:36:03 1.294
+++ configure.in 2001/04/29 01:44:00
@@ -259,7 +259,7 @@
if test "$pthreadh" = "1"; then
APR_CHECK_PTHREAD_GETSPECIFIC_TWO_ARGS
APR_CHECK_PTHREAD_ATTR_GETDETACHSTATE_ONE_ARG
- AC_CHECK_FUNCS(pthread_key_delete)
+ AC_CHECK_FUNCS(pthread_key_delete pthread_rwlock_init)
fi
fi
Index: include/apr_lock.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_lock.h,v
retrieving revision 1.23
diff -u -r1.23 apr_lock.h
--- include/apr_lock.h 2001/02/16 04:15:46 1.23
+++ include/apr_lock.h 2001/04/29 01:44:01
@@ -71,6 +71,8 @@
typedef enum {APR_MUTEX, APR_READWRITE} apr_locktype_e;
+typedef enum {APR_READER, APR_WRITER} apr_readerwriter_e;
+
typedef struct apr_lock_t apr_lock_t;
/* Function definitions */
@@ -110,6 +112,15 @@
* @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_readerwriter_e type)
+ */
+APR_DECLARE(apr_status_t) apr_lock_acquire_rw(apr_lock_t *lock,
+ apr_readerwriter_e type);
/**
* Unlock a protected region.
Index: include/arch/unix/locks.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/unix/locks.h,v
retrieving revision 1.26
diff -u -r1.26 locks.h
--- include/arch/unix/locks.h 2001/02/25 20:39:32 1.26
+++ include/arch/unix/locks.h 2001/04/29 01:44:01
@@ -130,9 +130,12 @@
#if APR_USE_PTHREAD_SERIALIZE
pthread_mutex_t *intraproc;
#endif
+#ifdef HAVE_PTHREAD_RWLOCK_INIT
+ pthread_rwlock_t rwlock;
#endif
+#endif
/* At some point, we should do a scope for both inter and intra process
- * locking here. Something like pthread_mutex with PTHREAD_PROCESS_SHARED
+ * locking here. Something like pthread_mutex with PTHREAD_PROCESS_SHARED
*/
};
@@ -149,8 +152,8 @@
apr_status_t apr_unix_unlock_inter(struct apr_lock_t *lock);
apr_status_t apr_unix_destroy_inter_lock(struct apr_lock_t *lock);
-apr_status_t apr_unix_child_init_lock(struct apr_lock_t **lock, apr_pool_t *cont,
- const char *fname);
+apr_status_t apr_unix_child_init_lock(struct apr_lock_t **lock,
+ apr_pool_t *cont, const char *fname);
#endif /* LOCKS_H */
Index: locks/beos/locks.c
===================================================================
RCS file: /home/cvspublic/apr/locks/beos/locks.c,v
retrieving revision 1.26
diff -u -r1.26 locks.c
--- locks/beos/locks.c 2001/02/16 04:15:53 1.26
+++ locks/beos/locks.c 2001/04/29 01:44:01
@@ -103,6 +103,11 @@
return APR_SUCCESS;
}
+apr_status_t apr_lock_acquire_rw(apr_lock_t *lock, apr_readerwriter_e e)
+{
+ return APR_ENOTIMPL;
+}
+
apr_status_t apr_lock_release(apr_lock_t *lock)
{
apr_status_t stat;
Index: locks/os2/locks.c
===================================================================
RCS file: /home/cvspublic/apr/locks/os2/locks.c,v
retrieving revision 1.27
diff -u -r1.27 locks.c
--- locks/os2/locks.c 2001/03/19 12:43:25 1.27
+++ locks/os2/locks.c 2001/04/29 01:44:02
@@ -142,7 +142,10 @@
return APR_OS2_STATUS(rc);
}
-
+apr_status_t apr_lock_acquire_rw(apr_lock_t *lock, apr_readerwriter_e e)
+{
+ return APR_ENOTIMPL;
+}
apr_status_t apr_lock_release(apr_lock_t *lock)
{
Index: locks/unix/locks.c
===================================================================
RCS file: /home/cvspublic/apr/locks/unix/locks.c,v
retrieving revision 1.46
diff -u -r1.46 locks.c
--- locks/unix/locks.c 2001/02/16 04:15:55 1.46
+++ locks/unix/locks.c 2001/04/29 01:44:02
@@ -68,6 +68,9 @@
new->cntxt = cont;
new->type = type;
new->scope = scope;
+ switch (new->type)
+ {
+ case APR_MUTEX:
#if (APR_USE_FCNTL_SERIALIZE) || (APR_USE_FLOCK_SERIALIZE)
/* file-based serialization primitives */
if (scope != APR_INTRAPROCESS) {
@@ -97,6 +100,18 @@
return stat;
}
}
+ break;
+ case APR_READWRITE:
+#ifdef HAVE_PTHREAD_RWLOCK_INIT
+ if (scope != APR_INTRAPROCESS)
+ return APR_ENOTIMPL;
+ pthread_rwlock_init(&new->rwlock, NULL);
+ break;
+#else
+ return APR_ENOTIMPL;
+#endif
+ }
+
*lock = new;
return APR_SUCCESS;
}
@@ -104,6 +119,10 @@
apr_status_t apr_lock_acquire(apr_lock_t *lock)
{
apr_status_t stat;
+
+ switch (lock->type)
+ {
+ case APR_MUTEX:
#if APR_PROCESS_LOCK_IS_GLOBAL /* don't need intra lock for APR_LOCKALL */
if (lock->scope == APR_INTRAPROCESS) {
#else
@@ -122,13 +141,49 @@
return stat;
}
}
+ break;
+ case APR_READWRITE:
+ return APR_ENOTIMPL;
+ }
+
return APR_SUCCESS;
}
+apr_status_t apr_lock_acquire_rw(apr_lock_t *lock, apr_readerwriter_e e)
+{
+ apr_status_t stat = APR_SUCCESS;
+
+ switch (lock->type)
+ {
+ case APR_MUTEX:
+ return APR_ENOTIMPL;
+ case APR_READWRITE:
+#ifdef HAVE_PTHREAD_RWLOCK_INIT
+ switch (e)
+ {
+ case APR_READER:
+ stat = pthread_rwlock_rdlock(&lock->rwlock);
+ break;
+ case APR_WRITER:
+ stat = pthread_rwlock_wrlock(&lock->rwlock);
+ break;
+ }
+ break;
+#else
+ return APR_ENOTIMPL;
+#endif
+ }
+
+ return stat;
+}
+
apr_status_t apr_lock_release(apr_lock_t *lock)
{
apr_status_t stat;
+ switch (lock->type)
+ {
+ case APR_MUTEX:
#if APR_PROCESS_LOCK_IS_GLOBAL /* don't need intra lock for APR_LOCKALL */
if (lock->scope == APR_INTRAPROCESS) {
#else
@@ -147,12 +202,27 @@
return stat;
}
}
+ break;
+ case APR_READWRITE:
+#ifdef HAVE_PTHREAD_RWLOCK_INIT
+ if ((stat = pthread_rwlock_unlock(&lock->rwlock)) != 0)
+ return stat;
+ break;
+#else
+ return APR_ENOTIMPL;
+#endif
+ }
+
return APR_SUCCESS;
}
apr_status_t apr_lock_destroy(apr_lock_t *lock)
{
apr_status_t stat;
+
+ switch (lock->type)
+ {
+ case APR_MUTEX:
#if APR_PROCESS_LOCK_IS_GLOBAL /* don't need intra lock for APR_LOCKALL */
if (lock->scope == APR_INTRAPROCESS) {
#else
@@ -172,6 +242,16 @@
if ((stat = apr_unix_destroy_inter_lock(lock)) != APR_SUCCESS) {
return stat;
}
+ }
+ break;
+ case APR_READWRITE:
+#ifdef HAVE_PTHREAD_RWLOCK_INIT
+ if ((stat = pthread_rwlock_destroy(&lock->rwlock)) != 0)
+ return stat;
+ break;
+#else
+ return APR_ENOTIMPL;
+#endif
}
return APR_SUCCESS;
}
Index: locks/win32/locks.c
===================================================================
RCS file: /home/cvspublic/apr/locks/win32/locks.c,v
retrieving revision 1.36
diff -u -r1.36 locks.c
--- locks/win32/locks.c 2001/02/16 04:15:56 1.36
+++ locks/win32/locks.c 2001/04/29 01:44:02
@@ -131,6 +131,12 @@
return apr_get_os_error();
}
+APR_DECLARE(apr_status_t) apr_lock_acquire_rw(apr_lock_t *lock,
+ apr_readerwriter_e e)
+{
+ return APR_ENOTIMPL;
+}
+
APR_DECLARE(apr_status_t) apr_lock_release(apr_lock_t *lock)
{
if (lock->scope == APR_INTRAPROCESS) {
Re: WriteLock ++ ;)
Posted by Christian Gross <Ch...@yahoo.de>.
>I think it's a matter of preference. I interpreted it as you want to
>acquire a lock, but you need to choose which type of lock you want
>receive. Again, no big deal. At this point, I think the fewer functions
>added, the better. And, in the grand scheme of things, a switch statement
>isn't going to cost THAT much. IMHO, the more readable the code is,
>the better. If you want raw speed with the fewest lines of code, you
>sure won't be using APR.
>
Again I agree with you and to me it does not matter which way. To be
honest either way is ok with me. The question is how do we decide?
How about we use the APR random function to flip a coin? ;)
Anyone on the Apache APR team wish to make a comment? Justin, if you
are please excuse my ignorance.
>In my implementation on POSIX platforms, it actually calls
>apr_lock_acquire_rw(lock, APR_READER) if you call apr_lock_acquire
>on a APR_READWRITE lock. You could make the case that they really
>want a writer lock. Or, I think APR_ENOTIMPL suits as well. Another
>gray area.
>
Regarding the comment that the lock request is really a writer lock,
yes that case could be made. Actually that is what I was thinking in
my mind. I decided against it because when the lock is created and it
is a simple lock certain structures are not filled in. But yes I
agree another gray area.
>Cross process? I'm ignoring the scope of the RW locks since POSIX
>RW locks *can't* be implemented cross-process. fcntl() locks can be
>cross-process. Can Windows rw locks be cross-process (oh, does the
>kernel somehow get involved)? I thought about returning APR_ENOTIMPL
>when they specified the scope, but I thought it would be better to
>just ignore it. Hmm.
>
On the Windows platform the locks can be cross-process. Hence right
now there is the problem that the RW locks will break between
processes. If they cannot be supported on the POSIX platform then
return APR_ENOTIMPL probably would be a good thing.
>We should probably integrate our patches into one to make it easier
>on others...
>
Ah, yes I agree with you. So how about we do the following. One
function, but return an error when using the wrong lock acquire
function is called?
All OK? Any comments?
>Two platforms down, two to go. -- justin
Yupe....
Christian
Re: WriteLock ++ ;)
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sat, Apr 28, 2001 at 07:29:02PM -0400, Christian Gross wrote:
> I just looked at the UNIX based RW lock stuff. There is one
> difference between what I uploaded and what Justin uploaded. He used
> one method call to acquire a reader or writer. I used two. Which one
> is right?
>
> I used two function calls because I thought it would keep things as
> simple and fast as possible. I looked in the UNIX code and within the
> apr_lock_acquire_rw routine there is a switch to test which type of
> lock to acquire. I would tend to say two method calls would be faster
> (OK it is splitting hairs ;) )
I think it's a matter of preference. I interpreted it as you want to
acquire a lock, but you need to choose which type of lock you want
receive. Again, no big deal. At this point, I think the fewer functions
added, the better. And, in the grand scheme of things, a switch statement
isn't going to cost THAT much. IMHO, the more readable the code is,
the better. If you want raw speed with the fewest lines of code, you
sure won't be using APR.
> Another question that I have is follows.
>
> A user creates a regular lock and calls the RW lock routines or
> creates a RW lock and calls the regular lock routines. What happens?
> In the Windows PATCH I decided to return an error saying this is not
> possible. However because of that I had to modify the apr_errno.h
> file to include an extra error code.
In my implementation on POSIX platforms, it actually calls
apr_lock_acquire_rw(lock, APR_READER) if you call apr_lock_acquire
on a APR_READWRITE lock. You could make the case that they really
want a writer lock. Or, I think APR_ENOTIMPL suits as well. Another
gray area.
> BTW I just caught an error in the Windows routines in that when the
> lock is cross process my reader writer will fail. Need to fix that...
Cross process? I'm ignoring the scope of the RW locks since POSIX
RW locks *can't* be implemented cross-process. fcntl() locks can be
cross-process. Can Windows rw locks be cross-process (oh, does the
kernel somehow get involved)? I thought about returning APR_ENOTIMPL
when they specified the scope, but I thought it would be better to
just ignore it. Hmm.
We should probably integrate our patches into one to make it easier
on others...
Two platforms down, two to go. -- justin