You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by bo...@apache.org on 2007/05/12 14:31:19 UTC
svn commit: r537399 - in /apr/apr/branches/1.2.x: CHANGES
file_io/unix/filedup.c file_io/unix/open.c file_io/unix/readwrite.c
file_io/unix/seek.c include/arch/unix/apr_arch_file_io.h
Author: bojan
Date: Sat May 12 05:31:16 2007
New Revision: 537399
URL: http://svn.apache.org/viewvc?view=rev&rev=537399
Log:
Backport r537393 from the trunk.
Improve thread safety of assorted file_io functions.
Patches by Davi Arnaut.
Modified:
apr/apr/branches/1.2.x/CHANGES
apr/apr/branches/1.2.x/file_io/unix/filedup.c
apr/apr/branches/1.2.x/file_io/unix/open.c
apr/apr/branches/1.2.x/file_io/unix/readwrite.c
apr/apr/branches/1.2.x/file_io/unix/seek.c
apr/apr/branches/1.2.x/include/arch/unix/apr_arch_file_io.h
Modified: apr/apr/branches/1.2.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/CHANGES?view=diff&rev=537399&r1=537398&r2=537399
==============================================================================
--- apr/apr/branches/1.2.x/CHANGES (original)
+++ apr/apr/branches/1.2.x/CHANGES Sat May 12 05:31:16 2007
@@ -1,5 +1,8 @@
Changes for APR 1.2.9
+ *) Improve thread safety of assorted file_io functions.
+ PR 42400. [Davi Arnaut <davi haxent.com.br>]
+
*) Fix file pointer position calculation in apr_file_writev() on
buffered file. PR 40963. [Davi Arnaut <davi haxent.com.br>]
Modified: apr/apr/branches/1.2.x/file_io/unix/filedup.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/file_io/unix/filedup.c?view=diff&rev=537399&r1=537398&r2=537399
==============================================================================
--- apr/apr/branches/1.2.x/file_io/unix/filedup.c (original)
+++ apr/apr/branches/1.2.x/file_io/unix/filedup.c Sat May 12 05:31:16 2007
@@ -55,7 +55,7 @@
#if APR_HAS_THREADS
if ((*new_file)->buffered && !(*new_file)->thlock && old_file->thlock) {
apr_thread_mutex_create(&((*new_file)->thlock),
- APR_THREAD_MUTEX_DEFAULT, p);
+ APR_THREAD_MUTEX_NESTED, p);
}
#endif
/* As above, only create the buffer if we haven't already
@@ -131,7 +131,7 @@
#if APR_HAS_THREADS
if (old_file->thlock) {
apr_thread_mutex_create(&((*new_file)->thlock),
- APR_THREAD_MUTEX_DEFAULT, p);
+ APR_THREAD_MUTEX_NESTED, p);
apr_thread_mutex_destroy(old_file->thlock);
}
#endif /* APR_HAS_THREADS */
Modified: apr/apr/branches/1.2.x/file_io/unix/open.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/file_io/unix/open.c?view=diff&rev=537399&r1=537398&r2=537399
==============================================================================
--- apr/apr/branches/1.2.x/file_io/unix/open.c (original)
+++ apr/apr/branches/1.2.x/file_io/unix/open.c Sat May 12 05:31:16 2007
@@ -122,7 +122,7 @@
#if APR_HAS_THREADS
if ((flag & APR_BUFFERED) && (flag & APR_XTHREAD)) {
rv = apr_thread_mutex_create(&thlock,
- APR_THREAD_MUTEX_DEFAULT, pool);
+ APR_THREAD_MUTEX_NESTED, pool);
if (rv) {
return rv;
}
@@ -244,7 +244,7 @@
if ((*file)->flags & APR_XTHREAD) {
apr_status_t rv;
rv = apr_thread_mutex_create(&((*file)->thlock),
- APR_THREAD_MUTEX_DEFAULT, pool);
+ APR_THREAD_MUTEX_NESTED, pool);
if (rv) {
return rv;
}
Modified: apr/apr/branches/1.2.x/file_io/unix/readwrite.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/file_io/unix/readwrite.c?view=diff&rev=537399&r1=537398&r2=537399
==============================================================================
--- apr/apr/branches/1.2.x/file_io/unix/readwrite.c (original)
+++ apr/apr/branches/1.2.x/file_io/unix/readwrite.c Sat May 12 05:31:16 2007
@@ -92,19 +92,10 @@
}
if (thefile->buffered) {
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_lock(thefile->thlock);
- }
-#endif
-
+ file_lock(thefile);
rv = file_read_buffered(thefile, buf, nbytes);
+ file_unlock(thefile);
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_unlock(thefile->thlock);
- }
-#endif
return rv;
}
else {
@@ -162,11 +153,7 @@
int blocksize;
int size = *nbytes;
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_lock(thefile->thlock);
- }
-#endif
+ file_lock(thefile);
if ( thefile->direction == 0 ) {
/* Position file pointer for writing at the offset we are
@@ -192,11 +179,8 @@
size -= blocksize;
}
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_unlock(thefile->thlock);
- }
-#endif
+ file_unlock(thefile);
+
return rv;
}
else {
@@ -242,13 +226,16 @@
APR_DECLARE(apr_status_t) apr_file_writev(apr_file_t *thefile, const struct iovec *vec,
apr_size_t nvec, apr_size_t *nbytes)
{
+ apr_status_t rv;
#ifdef HAVE_WRITEV
int bytes;
-#endif
+
+ file_lock(thefile);
if (thefile->buffered) {
apr_status_t rv = apr_file_flush(thefile);
if (rv != APR_SUCCESS) {
+ file_unlock(thefile);
return rv;
}
if (thefile->direction == 0) {
@@ -263,15 +250,17 @@
}
}
-#ifdef HAVE_WRITEV
if ((bytes = writev(thefile->filedes, vec, nvec)) < 0) {
*nbytes = 0;
- return errno;
+ rv = errno;
}
else {
*nbytes = bytes;
- return APR_SUCCESS;
+ rv = APR_SUCCESS;
}
+
+ file_unlock(thefile);
+ return rv;
#else
/**
* The problem with trying to output the entire iovec is that we cannot
@@ -319,24 +308,29 @@
APR_DECLARE(apr_status_t) apr_file_flush(apr_file_t *thefile)
{
+ apr_status_t rv = APR_SUCCESS;
+
if (thefile->buffered) {
- apr_int64_t written = 0;
+ file_lock(thefile);
if (thefile->direction == 1 && thefile->bufpos) {
+ apr_int64_t written = 0;
do {
written = write(thefile->filedes, thefile->buffer, thefile->bufpos);
} while (written == (apr_int64_t)-1 && errno == EINTR);
if (written == (apr_int64_t)-1) {
- return errno;
+ rv = errno;
+ } else {
+ thefile->filePtr += written;
+ thefile->bufpos = 0;
}
- thefile->filePtr += written;
- thefile->bufpos = 0;
}
+ file_unlock(thefile);
}
/* There isn't anything to do if we aren't buffering the output
* so just return success.
*/
- return APR_SUCCESS;
+ return rv;
}
APR_DECLARE(apr_status_t) apr_file_gets(char *str, int len, apr_file_t *thefile)
@@ -357,20 +351,12 @@
*/
if (thefile->buffered) {
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_lock(thefile->thlock);
- }
-#endif
+ file_lock(thefile);
if (thefile->direction == 1) {
rv = apr_file_flush(thefile);
if (rv) {
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_unlock(thefile->thlock);
- }
-#endif
+ file_unlock(thefile);
return rv;
}
@@ -399,11 +385,7 @@
++str;
}
-#if APR_HAS_THREADS
- if (thefile->thlock) {
- apr_thread_mutex_unlock(thefile->thlock);
- }
-#endif
+ file_unlock(thefile);
}
else {
while (str < final) { /* leave room for trailing '\0' */
Modified: apr/apr/branches/1.2.x/file_io/unix/seek.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/file_io/unix/seek.c?view=diff&rev=537399&r1=537398&r2=537399
==============================================================================
--- apr/apr/branches/1.2.x/file_io/unix/seek.c (original)
+++ apr/apr/branches/1.2.x/file_io/unix/seek.c Sat May 12 05:31:16 2007
@@ -59,6 +59,8 @@
int rc = EINVAL;
apr_finfo_t finfo;
+ file_lock(thefile);
+
switch (where) {
case APR_SET:
rc = setptr(thefile, *offset);
@@ -76,6 +78,9 @@
}
*offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
+
+ file_unlock(thefile);
+
return rc;
}
else {
Modified: apr/apr/branches/1.2.x/include/arch/unix/apr_arch_file_io.h
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/arch/unix/apr_arch_file_io.h?view=diff&rev=537399&r1=537398&r2=537399
==============================================================================
--- apr/apr/branches/1.2.x/include/arch/unix/apr_arch_file_io.h (original)
+++ apr/apr/branches/1.2.x/include/arch/unix/apr_arch_file_io.h Sat May 12 05:31:16 2007
@@ -110,6 +110,14 @@
#endif
};
+#if APR_HAS_THREADS
+#define file_lock(f) {if ((f)->thlock) apr_thread_mutex_lock((f)->thlock);}
+#define file_unlock(f) {if ((f)->thlock) apr_thread_mutex_unlock((f)->thlock);}
+#else
+#define file_lock(f) {}
+#define file_unlock(f) {}
+#endif
+
#if APR_HAS_LARGE_FILES && defined(_LARGEFILE64_SOURCE)
#define stat(f,b) stat64(f,b)
#define lstat(f,b) lstat64(f,b)