apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bo...@apache.org
Subject svn commit: r529830 - in /apr/apr/branches/1.2.x: CHANGES file_io/unix/readwrite.c test/testfile.c
Date Wed, 18 Apr 2007 02:14:17 GMT
Author: bojan
Date: Tue Apr 17 19:14:16 2007
New Revision: 529830

URL: http://svn.apache.org/viewvc?view=rev&rev=529830
Log:
Backport r524823, r524822, r524821, r524355 from the trunk
Fix deadlock in apr_file_gets() for a file opened with both the
APR_BUFFERED and APR_XTHREAD flags

Modified:
    apr/apr/branches/1.2.x/CHANGES
    apr/apr/branches/1.2.x/file_io/unix/readwrite.c
    apr/apr/branches/1.2.x/test/testfile.c

Modified: apr/apr/branches/1.2.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/CHANGES?view=diff&rev=529830&r1=529829&r2=529830
==============================================================================
--- apr/apr/branches/1.2.x/CHANGES (original)
+++ apr/apr/branches/1.2.x/CHANGES Tue Apr 17 19:14:16 2007
@@ -1,6 +1,7 @@
 Changes for APR 1.2.9
 
-
+  *) Fix deadlock in apr_file_gets() for a file opened with both the
+     APR_BUFFERED and APR_XTHREAD flags.  [Bojan Smojver]
 
 Changes for APR 1.2.8
 

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=529830&r1=529829&r2=529830
==============================================================================
--- apr/apr/branches/1.2.x/file_io/unix/readwrite.c (original)
+++ apr/apr/branches/1.2.x/file_io/unix/readwrite.c Tue Apr 17 19:14:16 2007
@@ -25,6 +25,62 @@
 #define USE_WAIT_FOR_IO
 #endif
 
+static apr_status_t file_read_buffered(apr_file_t *thefile, void *buf,
+                                       apr_size_t *nbytes)
+{
+    apr_ssize_t rv;
+    char *pos = (char *)buf;
+    apr_uint64_t blocksize;
+    apr_uint64_t size = *nbytes;
+
+    if (thefile->direction == 1) {
+        rv = apr_file_flush(thefile);
+        if (rv) {
+            return rv;
+        }
+        thefile->bufpos = 0;
+        thefile->direction = 0;
+        thefile->dataRead = 0;
+    }
+
+    rv = 0;
+    if (thefile->ungetchar != -1) {
+        *pos = (char)thefile->ungetchar;
+        ++pos;
+        --size;
+        thefile->ungetchar = -1;
+    }
+    while (rv == 0 && size > 0) {
+        if (thefile->bufpos >= thefile->dataRead) {
+            int bytesread = read(thefile->filedes, thefile->buffer, APR_FILE_BUFSIZE);
+            if (bytesread == 0) {
+                thefile->eof_hit = TRUE;
+                rv = APR_EOF;
+                break;
+            }
+            else if (bytesread == -1) {
+                rv = errno;
+                break;
+            }
+            thefile->dataRead = bytesread;
+            thefile->filePtr += thefile->dataRead;
+            thefile->bufpos = 0;
+        }
+
+        blocksize = size > thefile->dataRead - thefile->bufpos ? thefile->dataRead
- thefile->bufpos : size;
+        memcpy(pos, thefile->buffer + thefile->bufpos, blocksize);
+        thefile->bufpos += blocksize;
+        pos += blocksize;
+        size -= blocksize;
+    }
+
+    *nbytes = pos - (char *)buf;
+    if (*nbytes) {
+        rv = 0;
+    }
+    return rv;
+}
+
 APR_DECLARE(apr_status_t) apr_file_read(apr_file_t *thefile, void *buf, apr_size_t *nbytes)
 {
     apr_ssize_t rv;
@@ -36,66 +92,14 @@
     }
 
     if (thefile->buffered) {
-        char *pos = (char *)buf;
-        apr_uint64_t blocksize;
-        apr_uint64_t size = *nbytes;
-
 #if APR_HAS_THREADS
         if (thefile->thlock) {
             apr_thread_mutex_lock(thefile->thlock);
         }
 #endif
 
-        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
-                return rv;
-            }
-            thefile->bufpos = 0;
-            thefile->direction = 0;
-            thefile->dataRead = 0;
-        }
+        rv = file_read_buffered(thefile, buf, nbytes);
 
-        rv = 0;
-        if (thefile->ungetchar != -1) {
-            *pos = (char)thefile->ungetchar;
-            ++pos;
-            --size;
-            thefile->ungetchar = -1;
-        }
-        while (rv == 0 && size > 0) {
-            if (thefile->bufpos >= thefile->dataRead) {
-                int bytesread = read(thefile->filedes, thefile->buffer, APR_FILE_BUFSIZE);
-                if (bytesread == 0) {
-                    thefile->eof_hit = TRUE;
-                    rv = APR_EOF;
-                    break;
-                }
-                else if (bytesread == -1) {
-                    rv = errno;
-                    break;
-                }
-                thefile->dataRead = bytesread;
-                thefile->filePtr += thefile->dataRead;
-                thefile->bufpos = 0;
-            }
-
-            blocksize = size > thefile->dataRead - thefile->bufpos ? thefile->dataRead
- thefile->bufpos : size;
-            memcpy(pos, thefile->buffer + thefile->bufpos, blocksize);
-            thefile->bufpos += blocksize;
-            pos += blocksize;
-            size -= blocksize;
-        }
-
-        *nbytes = pos - (char *)buf;
-        if (*nbytes) {
-            rv = 0;
-        }
 #if APR_HAS_THREADS
         if (thefile->thlock) {
             apr_thread_mutex_unlock(thefile->thlock);
@@ -364,7 +368,7 @@
             }
             else {
                 nbytes = 1;
-                rv = apr_file_read(thefile, str, &nbytes);
+                rv = file_read_buffered(thefile, str, &nbytes);
                 if (rv != APR_SUCCESS) {
                     break;
                 }

Modified: apr/apr/branches/1.2.x/test/testfile.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/test/testfile.c?view=diff&rev=529830&r1=529829&r2=529830
==============================================================================
--- apr/apr/branches/1.2.x/test/testfile.c (original)
+++ apr/apr/branches/1.2.x/test/testfile.c Tue Apr 17 19:14:16 2007
@@ -386,6 +386,29 @@
     apr_file_close(f);
 }
 
+static void test_gets_buffered(abts_case *tc, void *data)
+{
+    apr_file_t *f = NULL;
+    apr_status_t rv;
+    char *str = apr_palloc(p, 256);
+
+    /* This will deadlock gets before the r524355 fix. */
+    rv = apr_file_open(&f, FILENAME, APR_READ|APR_BUFFERED|APR_XTHREAD, 0, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_file_gets(str, 256, f);
+    /* Only one line in the test file, so APR will encounter EOF on the first
+     * call to gets, but we should get APR_SUCCESS on this call and
+     * APR_EOF on the next.
+     */
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_STR_EQUAL(tc, TESTSTR, str);
+    rv = apr_file_gets(str, 256, f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    ABTS_STR_EQUAL(tc, "", str);
+    apr_file_close(f);
+}
+
 static void test_bigread(abts_case *tc, void *data)
 {
     apr_file_t *f = NULL;
@@ -853,6 +876,7 @@
     abts_run_test(suite, test_getc, NULL);
     abts_run_test(suite, test_ungetc, NULL);
     abts_run_test(suite, test_gets, NULL);
+    abts_run_test(suite, test_gets_buffered, NULL);
     abts_run_test(suite, test_puts, NULL);
     abts_run_test(suite, test_writev, NULL);
     abts_run_test(suite, test_writev_full, NULL);



Mime
View raw message