apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bo...@apache.org
Subject svn commit: r537393 - in /apr/apr/trunk: CHANGES file_io/unix/buffer.c file_io/unix/filedup.c file_io/unix/filestat.c file_io/unix/open.c file_io/unix/readwrite.c file_io/unix/seek.c include/arch/unix/apr_arch_file_io.h
Date Sat, 12 May 2007 11:47:30 GMT
Author: bojan
Date: Sat May 12 04:47:29 2007
New Revision: 537393

URL: http://svn.apache.org/viewvc?view=rev&rev=537393
Log:
Improve thread safety of assorted file_io functions.
Patches by Davi Arnaut.

Modified:
    apr/apr/trunk/CHANGES
    apr/apr/trunk/file_io/unix/buffer.c
    apr/apr/trunk/file_io/unix/filedup.c
    apr/apr/trunk/file_io/unix/filestat.c
    apr/apr/trunk/file_io/unix/open.c
    apr/apr/trunk/file_io/unix/readwrite.c
    apr/apr/trunk/file_io/unix/seek.c
    apr/apr/trunk/include/arch/unix/apr_arch_file_io.h

Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/CHANGES (original)
+++ apr/apr/trunk/CHANGES Sat May 12 04:47:29 2007
@@ -1,5 +1,8 @@
 Changes for APR 1.3.0
 
+  *) Improve thread safety of assorted file_io functions.
+     PR 42400.  [Davi Arnaut <davi haxent.com.br>]
+
   *) Fix deadlock in apr_file_gets() for a file opened with both the
      APR_BUFFERED and APR_XTHREAD flags.  [Bojan Smojver, Joe Orton]
 

Modified: apr/apr/trunk/file_io/unix/buffer.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/buffer.c?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/file_io/unix/buffer.c (original)
+++ apr/apr/trunk/file_io/unix/buffer.c Sat May 12 04:47:29 2007
@@ -24,21 +24,13 @@
 {
     apr_status_t rv;
 
-#if APR_HAS_THREADS
-     if (file->thlock) {
-         apr_thread_mutex_lock(file->thlock);
-     }
-#endif
- 
+    file_lock(file);
+
     if(file->buffered) {
         /* Flush the existing buffer */
         rv = apr_file_flush(file);
         if (rv != APR_SUCCESS) {
-#if APR_HAS_THREADS
-            if (file->thlock) {
-                apr_thread_mutex_unlock(file->thlock);
-            }
-#endif
+            file_unlock(file);
             return rv;
         }
     }
@@ -56,12 +48,8 @@
              */
             file->buffered = 0;
     }
-    
-#if APR_HAS_THREADS
-    if (file->thlock) {
-        apr_thread_mutex_unlock(file->thlock);
-    }
-#endif
+
+    file_unlock(file);
 
     return APR_SUCCESS;
 }

Modified: apr/apr/trunk/file_io/unix/filedup.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/filedup.c?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/file_io/unix/filedup.c (original)
+++ apr/apr/trunk/file_io/unix/filedup.c Sat May 12 04:47:29 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
@@ -133,7 +133,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/trunk/file_io/unix/filestat.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/filestat.c?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/file_io/unix/filestat.c (original)
+++ apr/apr/trunk/file_io/unix/filestat.c Sat May 12 04:47:29 2007
@@ -107,7 +107,6 @@
     struct_stat info;
 
     if (thefile->buffered) {
-        /* XXX: flush here is not mutex protected */
         apr_status_t rv = apr_file_flush(thefile);
         if (rv != APR_SUCCESS)
             return rv;

Modified: apr/apr/trunk/file_io/unix/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/open.c?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/file_io/unix/open.c (original)
+++ apr/apr/trunk/file_io/unix/open.c Sat May 12 04:47:29 2007
@@ -32,7 +32,6 @@
     apr_status_t flush_rv = APR_SUCCESS, rv = APR_SUCCESS;
 
     if (file->buffered) {
-        /* XXX: flush here is not mutex protected */
         flush_rv = apr_file_flush(file);
     }
     if (close(file->filedes) == 0) {
@@ -123,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;
         }
@@ -247,7 +246,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/trunk/file_io/unix/readwrite.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/readwrite.c?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/file_io/unix/readwrite.c (original)
+++ apr/apr/trunk/file_io/unix/readwrite.c Sat May 12 04:47:29 2007
@@ -93,19 +93,9 @@
     }
 
     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);
-
-#if APR_HAS_THREADS
-        if (thefile->thlock) {
-            apr_thread_mutex_unlock(thefile->thlock);
-        }
-#endif
+        file_unlock(thefile);
         return rv;
     }
     else {
@@ -163,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 
@@ -193,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 {
@@ -243,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
     apr_ssize_t bytes;
-#endif
+
+    file_lock(thefile);
 
     if (thefile->buffered) {
-        apr_status_t rv = apr_file_flush(thefile);
+        rv = apr_file_flush(thefile);
         if (rv != APR_SUCCESS) {
+            file_unlock(thefile);
             return rv;
         }
         if (thefile->direction == 0) {
@@ -263,15 +249,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,7 +307,10 @@
 
 APR_DECLARE(apr_status_t) apr_file_flush(apr_file_t *thefile)
 {
+    apr_status_t rv = APR_SUCCESS;
+
     if (thefile->buffered) {
+        file_lock(thefile);
         if (thefile->direction == 1 && thefile->bufpos) {
             apr_ssize_t written;
 
@@ -327,16 +318,18 @@
                 written = write(thefile->filedes, thefile->buffer, thefile->bufpos);
             } while (written == -1 && errno == EINTR);
             if (written == -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)
@@ -356,21 +349,12 @@
      * and skip over the apr_file_read calls.
      */
     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;
             }
 
@@ -398,12 +382,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/trunk/file_io/unix/seek.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/seek.c?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/file_io/unix/seek.c (original)
+++ apr/apr/trunk/file_io/unix/seek.c Sat May 12 04:47:29 2007
@@ -22,7 +22,6 @@
     apr_status_t rv;
 
     if (thefile->direction == 1) {
-        /* XXX: flush here is not mutex protected */
         rv = apr_file_flush(thefile);
         if (rv) {
             return rv;
@@ -60,6 +59,8 @@
         int rc = EINVAL;
         apr_finfo_t finfo;
 
+        file_lock(thefile);
+
         switch (where) {
         case APR_SET:
             rc = setptr(thefile, *offset);
@@ -77,6 +78,9 @@
         }
 
         *offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
+
+        file_unlock(thefile);
+
         return rc;
     }
     else {

Modified: apr/apr/trunk/include/arch/unix/apr_arch_file_io.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/arch/unix/apr_arch_file_io.h?view=diff&rev=537393&r1=537392&r2=537393
==============================================================================
--- apr/apr/trunk/include/arch/unix/apr_arch_file_io.h (original)
+++ apr/apr/trunk/include/arch/unix/apr_arch_file_io.h Sat May 12 04:47:29 2007
@@ -113,6 +113,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)



Mime
View raw message