The attached patch introduces an unnested Win32 Event-based thread_mutex that is compatible with the assumption that a single thread may obtain a lock for another thread, then wait for the second thread to release it.
It further degrades to Mutex objects for nested Win9x locks, since we couldn't 'try' a thread_mutex based on CriticalSection objects before Windows NT.
I also attached a simple patch to illustrate the fix to mod_isapi with the change to the API. Hoping Sebastian could take a look as well.
If I don't hear objections, I'll commit later this afternoon. Comments welcome.
Bill
At 10:19 AM 5/29/2002, William A. Rowe, Jr. wrote:
Attached is a patch to introduce a new locking semantic to our thread_mutex.
Right now we presume the 'default' is unnested. Well, that saves 30% of the
processing time on Unix, but it would cripple Win32 (which gets a critical section
in 10 instructions or so IF there is no contention... and it's always nested...)
Sebastian tossed me a really interesting and invalid assumption I made when apr'izing mod_isapi...
At 09:38 AM 5/25/2002, Sebastian Hantsch wrote to me:[...] the current Async emulation won't work as designed:
This is a snippet from isapi_handler: comp = cid->completed; if (cid->completed && (rv == APR_SUCCESS)) { rv = apr_thread_mutex_lock(comp); } /* The completion port is now locked. When we regain the * lock, we may destroy the request. */ if (cid->completed && (rv == APR_SUCCESS)) { rv = apr_thread_mutex_lock(comp); } break;
apr_thread_mutex functions internally use Critical Sections on win32 platform.
In the Win32 SDK at
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/synchro_3xym.asp
is the following statement:
> After a thread has ownership of a critical section, it can make additional calls to
> EnterCriticalSection or TryEnterCriticalSection without blocking its execution. This
> prevents a thread from deadlocking itself while waiting for a critical section that it already owns.
So, the method currently used in mod_isapi.c does not block as supposed until a HSE_REQ_DONE_WITH_SESSION message arrives.
I would suggest to use SetEvent/CreateEvent/WaitForSingleObject as in 2.0.36, that synchronization methods work for me.
Index: include/apr_thread_mutex.h
===================================================================
RCS file: /home/cvs/apr/include/apr_thread_mutex.h,v
retrieving revision 1.10
diff -u -r1.10 apr_thread_mutex.h
--- include/apr_thread_mutex.h 9 Apr 2002 06:56:55 -0000 1.10
+++ include/apr_thread_mutex.h 29 May 2002 16:41:19 -0000
@@ -79,6 +79,7 @@
#define APR_THREAD_MUTEX_DEFAULT 0x0
#define APR_THREAD_MUTEX_NESTED 0x1
+#define APR_THREAD_MUTEX_UNNESTED 0x2
/* Delayed the include to avoid a circular reference */
#include "apr_pools.h"
@@ -89,10 +90,14 @@
* stored.
* @param flags Or'ed value of:
* <PRE>
- * APR_THREAD_MUTEX_DEFAULT normal lock behavior (non-recursive).
+ * APR_THREAD_MUTEX_DEFAULT platform-optimal lock behavior.
* APR_THREAD_MUTEX_NESTED enable nested (recursive) locks.
+ * APR_THREAD_MUTEX_UNNESTED disable nested locks (non-recursive).
* </PRE>
* @param pool the pool from which to allocate the mutex.
+ * @tip Be cautious in using APR_THREAD_MUTEX_DEFAULT. While this is the
+ * most optimial mutex based on a given platform's performance charateristics,
+ * it will behave as either a nested or an unnested lock.
*/
APR_DECLARE(apr_status_t) apr_thread_mutex_create(apr_thread_mutex_t **mutex,
unsigned int flags,
Index: include/arch/win32/thread_mutex.h
===================================================================
RCS file: /home/cvs/apr/include/arch/win32/thread_mutex.h,v
retrieving revision 1.4
diff -u -r1.4 thread_mutex.h
--- include/arch/win32/thread_mutex.h 9 Apr 2002 06:56:56 -0000 1.4
+++ include/arch/win32/thread_mutex.h 29 May 2002 16:41:19 -0000
@@ -57,9 +57,21 @@
#include "apr_pools.h"
+typedef enum thread_mutex_type {
+ thread_mutex_critical_section,
+ thread_mutex_unnested_event,
+ thread_mutex_nested_mutex
+} thread_mutex_type;
+
+/* handle applies only to unnested_event on all platforms
+ * and nested_mutex on Win9x only. Otherwise critical_section
+ * is used for NT nexted mutexes providing optimal performance.
+ */
struct apr_thread_mutex_t {
- apr_pool_t *pool;
- CRITICAL_SECTION section;
+ apr_pool_t *pool;
+ thread_mutex_type type;
+ HANDLE handle;
+ CRITICAL_SECTION section;
};
#endif /* THREAD_MUTEX_H */
Index: locks/beos/thread_mutex.c
===================================================================
RCS file: /home/cvs/apr/locks/beos/thread_mutex.c,v
retrieving revision 1.7
diff -u -r1.7 thread_mutex.c
--- locks/beos/thread_mutex.c 13 Mar 2002 20:39:20 -0000 1.7
+++ locks/beos/thread_mutex.c 29 May 2002 16:41:19 -0000
@@ -96,6 +96,10 @@
new_m->LockCount = 0;
new_m->Lock = stat;
new_m->pool = pool;
+
+ /* Optimal default is APR_THREAD_MUTEX_UNNESTED,
+ * no additional checks required for either flag.
+ */
new_m->nested = flags & APR_THREAD_MUTEX_NESTED;
apr_pool_cleanup_register(new_m->pool, (void *)new_m,
_thread_mutex_cleanup,
Index: locks/netware/thread_mutex.c
===================================================================
RCS file: /home/cvs/apr/locks/netware/thread_mutex.c,v
retrieving revision 1.7
diff -u -r1.7 thread_mutex.c
--- locks/netware/thread_mutex.c 13 Mar 2002 20:39:20 -0000 1.7
+++ locks/netware/thread_mutex.c 29 May 2002 16:41:19 -0000
@@ -73,6 +73,11 @@
{
apr_thread_mutex_t *new_mutex = NULL;
+ /* XXX: Implement _UNNESTED flavor and favor _DEFAULT for performance
+ */
+ if (flags & APR_THREAD_MUTEX_UNNESTED) {
+ return APR_ENOTIMPL;
+ }
new_mutex = (apr_thread_mutex_t *)apr_pcalloc(pool,
sizeof(apr_thread_mutex_t));
if(new_mutex ==NULL) {
@@ -80,7 +85,6 @@
}
new_mutex->pool = pool;
- /* FIXME: only use recursive locks if (flags & APR_THREAD_MUTEX_NESTED) */
new_mutex->mutex = NXMutexAlloc(NX_MUTEX_RECURSIVE, NULL, NULL);
if(new_mutex->mutex == NULL)
Index: locks/os2/thread_mutex.c
===================================================================
RCS file: /home/cvs/apr/locks/os2/thread_mutex.c,v
retrieving revision 1.6
diff -u -r1.6 thread_mutex.c
--- locks/os2/thread_mutex.c 13 Mar 2002 20:39:21 -0000 1.6
+++ locks/os2/thread_mutex.c 29 May 2002 16:41:19 -0000
@@ -69,6 +69,9 @@
+/* XXX: Need to respect APR_THREAD_MUTEX_[UN]NESTED flags argument
+ * or return APR_ENOTIMPL!!!
+ */
APR_DECLARE(apr_status_t) apr_thread_mutex_create(apr_thread_mutex_t **mutex,
unsigned int flags,
apr_pool_t *pool)
Index: locks/unix/thread_mutex.c
===================================================================
RCS file: /home/cvs/apr/locks/unix/thread_mutex.c,v
retrieving revision 1.10
diff -u -r1.10 thread_mutex.c
--- locks/unix/thread_mutex.c 28 Apr 2002 03:21:24 -0000 1.10
+++ locks/unix/thread_mutex.c 29 May 2002 16:41:19 -0000
@@ -89,6 +89,10 @@
}
new_mutex->pool = pool;
+
+ /* Optimal default is APR_THREAD_MUTEX_UNNESTED,
+ * no additional checks required for either flag.
+ */
new_mutex->nested = flags & APR_THREAD_MUTEX_NESTED;
if ((rv = pthread_mutexattr_init(&mattr))) {
Index: locks/win32/thread_mutex.c
===================================================================
RCS file: /home/cvs/apr/locks/win32/thread_mutex.c,v
retrieving revision 1.9
diff -u -r1.9 thread_mutex.c
--- locks/win32/thread_mutex.c 13 Mar 2002 20:39:22 -0000 1.9
+++ locks/win32/thread_mutex.c 29 May 2002 16:41:19 -0000
@@ -65,7 +65,12 @@
{
apr_thread_mutex_t *lock = data;
- DeleteCriticalSection(&lock->section);
+ if (lock->type == thread_mutex_critical_section) {
+ DeleteCriticalSection(&lock->section);
+ }
+ else {
+ CloseHandle(lock->handle);
+ }
return APR_SUCCESS;
}
@@ -73,12 +78,41 @@
unsigned int flags,
apr_pool_t *pool)
{
+ if (flags & APR_THREAD_MUTEX_UNNESTED) {
+ return APR_ENOTIMPL;
+ }
+
(*mutex) = (apr_thread_mutex_t *)apr_palloc(pool, sizeof(**mutex));
(*mutex)->pool = pool;
- /* FIXME: Implement nested (aka recursive) locks or use a native
- * win32 implementation if available. */
- InitializeCriticalSection(&(*mutex)->section);
+
+ if (flags & APR_THREAD_MUTEX_UNNESTED) {
+ /* Use an auto-reset signaled event, ready to accept one
+ * waiting thread.
+ */
+ (*mutex)->type = thread_mutex_unnested_event;
+ (*mutex)->handle = CreateEvent(NULL, FALSE, TRUE, NULL);
+ }
+ else {
+#if APR_HAS_UNICODE_FS
+ /* Critical Sections are terrific, performance-wise, on NT.
+ * On Win9x, we cannot 'try' on a critical section, so we
+ * use a [slower] mutex object, instead.
+ */
+ IF_WIN_OS_IS_UNICODE {
+ (*mutex)->type = thread_mutex_critical_section;
+ InitializeCriticalSection(&(*mutex)->section);
+ }
+#endif
+#if APR_HAS_ANSI_FS
+ ELSE_WIN_OS_IS_ANSI {
+ (*mutex)->type = thread_mutex_nested_mutex;
+ (*mutex)->handle = CreateMutex(NULL, FALSE, NULL);
+
+ }
+#endif
+ }
+
apr_pool_cleanup_register((*mutex)->pool, (*mutex), thread_mutex_cleanup,
apr_pool_cleanup_null);
return APR_SUCCESS;
@@ -86,24 +120,41 @@
APR_DECLARE(apr_status_t) apr_thread_mutex_lock(apr_thread_mutex_t *mutex)
{
- EnterCriticalSection(&mutex->section);
+ if (mutex->type == thread_mutex_critical_section) {
+ EnterCriticalSection(&mutex->section);
+ }
+ else {
+ WaitForSingleObject(mutex->handle, INFINITE);
+ }
return APR_SUCCESS;
}
APR_DECLARE(apr_status_t) apr_thread_mutex_trylock(apr_thread_mutex_t *mutex)
{
- if (apr_os_level < APR_WIN_NT) {
- return APR_ENOTIMPL;
- }
- if (TryEnterCriticalSection(&mutex->section)) {
- return APR_SUCCESS;
+ if (mutex->type == thread_mutex_critical_section) {
+ if (TryEnterCriticalSection(&mutex->section)) {
+ return APR_SUCCESS;
+ }
}
+ else {
+ if (WaitForSingleObject(mutex->handle, 0) != WAIT_TIMEOUT) {
+ return APR_SUCCESS;
+ }
+ }
return APR_EBUSY;
}
APR_DECLARE(apr_status_t) apr_thread_mutex_unlock(apr_thread_mutex_t *mutex)
{
- LeaveCriticalSection(&mutex->section);
+ if (mutex->type == thread_mutex_critical_section) {
+ LeaveCriticalSection(&mutex->section);
+ }
+ else if (mutex->type == thread_mutex_unnested_event) {
+ SetEvent(mutex->handle);
+ }
+ else if (mutex->type == thread_mutex_nested_mutex) {
+ ReleaseMutex(mutex->handle);
+ }
return APR_SUCCESS;
}
Index: modules/arch/win32/mod_isapi.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/arch/win32/mod_isapi.c,v
retrieving revision 1.77
diff -u -r1.77 mod_isapi.c
--- modules/arch/win32/mod_isapi.c 25 May 2002 15:31:27 -0000 1.77
+++ modules/arch/win32/mod_isapi.c 29 May 2002 16:42:27 -0000
@@ -1487,7 +1487,7 @@
apr_thread_mutex_t *comp;
rv = apr_thread_mutex_create(&cid->completed,
- APR_THREAD_MUTEX_DEFAULT,
+ APR_THREAD_MUTEX_UNNESTED,
r->pool);
comp = cid->completed;
if (cid->completed && (rv == APR_SUCCESS)) {
