Hi everyone,
Currently, there are some cases in the Windows implementation of the file
I/O that cause the unnecessary creation and acquisitions of the file mutex.
These cases include handling O_APPEND-style writes and the implementation
of the apr_file_buffer_set().
Creating and acquiring the file mutex is only required for files opened
with the APR_FOPEN_XTHREAD flag. Otherwise, concurrent operations
on the same apr_file_t instance should be serialized by the user, and
the mutex is not required.
This patch tweaks the implementation to only create and acquire/release
the mutex for files opened with APR_FOPEN_XTHREAD. The log message
is included in the beginning of the patch file.
Thanks,
Evgeny Kotkov
Win32: Create and use file mutex only for files opened with APR_FOPEN_XTHREAD.
There are some cases in the Windows implementation of the file I/O that
cause the unnecessary creation and acquisitions of the file mutex.
These cases include handling O_APPEND-style writes and the implementation
of the apr_file_buffer_set().
Creating and acquiring the file mutex is only required for files opened
with the APR_FOPEN_XTHREAD flag. Otherwise, concurrent operations on
the same apr_file_t instance should be serialized by the user, and
the mutex is not required.
This patch tweaks the implementation to only create and acquire/release
the mutex for files opened with APR_FOPEN_XTHREAD.
* file_io/win32/open.c
(apr_file_open, apr_os_file_put): Create the file mutex only
with APR_FOPEN_XTHREAD.
* file_io/win32/buffer.c
(apr_file_buffer_set): Use the file mutex only with APR_FOPEN_XTHREAD.
* file_io/win32/readwrite.c
(apr_file_write): Use the file mutex only with APR_FOPEN_XTHREAD.
Patch by: Evgeny Kotkov <evgeny.kotkov {at} visualsvn.com>
Index: file_io/win32/buffer.c
===================================================================
--- file_io/win32/buffer.c (revision 1806645)
+++ file_io/win32/buffer.c (working copy)
@@ -23,7 +23,9 @@ APR_DECLARE(apr_status_t) apr_file_buffer_set(apr_
{
apr_status_t rv;
- apr_thread_mutex_lock(file->mutex);
+ if (file->flags & APR_FOPEN_XTHREAD) {
+ apr_thread_mutex_lock(file->mutex);
+ }
if(file->buffered) {
/* Flush the existing buffer */
@@ -48,7 +50,9 @@ APR_DECLARE(apr_status_t) apr_file_buffer_set(apr_
file->buffered = 0;
}
- apr_thread_mutex_unlock(file->mutex);
+ if (file->flags & APR_FOPEN_XTHREAD) {
+ apr_thread_mutex_unlock(file->mutex);
+ }
return APR_SUCCESS;
}
Index: file_io/win32/open.c
===================================================================
--- file_io/win32/open.c (revision 1806645)
+++ file_io/win32/open.c (working copy)
@@ -452,8 +452,8 @@ APR_DECLARE(apr_status_t) apr_file_open(apr_file_t
(*new)->buffer = apr_palloc(pool, APR_FILE_DEFAULT_BUFSIZE);
(*new)->bufsize = APR_FILE_DEFAULT_BUFSIZE;
}
- /* Need the mutex to handled buffered and O_APPEND style file i/o */
- if ((*new)->buffered || (*new)->append) {
+ /* Need the mutex to share an apr_file_t across multiple threads */
+ if (flag & APR_FOPEN_XTHREAD) {
rv = apr_thread_mutex_create(&(*new)->mutex,
APR_THREAD_MUTEX_DEFAULT, pool);
if (rv) {
@@ -645,8 +645,7 @@ APR_DECLARE(apr_status_t) apr_os_file_put(apr_file
(*file)->buffer = apr_palloc(pool, APR_FILE_DEFAULT_BUFSIZE);
(*file)->bufsize = APR_FILE_DEFAULT_BUFSIZE;
}
-
- if ((*file)->append || (*file)->buffered) {
+ if (flags & APR_FOPEN_XTHREAD) {
apr_status_t rv;
rv = apr_thread_mutex_create(&(*file)->mutex,
APR_THREAD_MUTEX_DEFAULT, pool);
Index: file_io/win32/readwrite.c
===================================================================
--- file_io/win32/readwrite.c (revision 1806645)
+++ file_io/win32/readwrite.c (working copy)
@@ -412,20 +412,26 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_
if (thefile->append) {
apr_off_t offset = 0;
- /* apr_file_lock will mutex the file across processes.
- * The call to apr_thread_mutex_lock is added to avoid
- * a race condition between LockFile and WriteFile
- * that occasionally leads to deadlocked threads.
- */
- apr_thread_mutex_lock(thefile->mutex);
+ if (thefile->flags & APR_FOPEN_XTHREAD) {
+ /* apr_file_lock will mutex the file across processes.
+ * The call to apr_thread_mutex_lock is added to avoid
+ * a race condition between LockFile and WriteFile
+ * that occasionally leads to deadlocked threads.
+ */
+ apr_thread_mutex_lock(thefile->mutex);
+ }
rv = apr_file_lock(thefile, APR_FLOCK_EXCLUSIVE);
if (rv != APR_SUCCESS) {
- apr_thread_mutex_unlock(thefile->mutex);
+ if (thefile->flags & APR_FOPEN_XTHREAD) {
+ apr_thread_mutex_unlock(thefile->mutex);
+ }
return rv;
}
rv = apr_file_seek(thefile, APR_END, &offset);
if (rv != APR_SUCCESS) {
- apr_thread_mutex_unlock(thefile->mutex);
+ if (thefile->flags & APR_FOPEN_XTHREAD) {
+ apr_thread_mutex_unlock(thefile->mutex);
+ }
return rv;
}
}
@@ -440,7 +446,9 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_
}
if (thefile->append) {
apr_file_unlock(thefile);
- apr_thread_mutex_unlock(thefile->mutex);
+ if (thefile->flags & APR_FOPEN_XTHREAD) {
+ apr_thread_mutex_unlock(thefile->mutex);
+ }
}
}
if (rv) {
