Merged rr1559873 from trunk:

    Fix timing bug in parallel apr_dir_make_recursive on Windows.

    * file_io/win32/dir.c
      (dir_make_parent): When parent just got created, continue creating children.
      (apr_dir_make_recursive): Only handle EEXIST of the requested directory as
       success, not any ancestor.

    Patch by: rhuijben

    * test/testdir.c
      (test_mkdir_recurs_parallel): New multithreaded test case.
      (test_removeall): Clean up after test_mkdir_recurs_parallel.


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/branches/1.4.x@1559879 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 146 insertions, 17 deletions

diff --git a/file_io/win32/dir.c b/file_io/win32/dir.c
index f6eeb8725..5c30af8fd 100644
--- a/file_io/win32/dir.c
+++ b/file_io/win32/dir.c
@@ -316,8 +316,8 @@ static apr_status_t dir_make_parent(char *path,
     if (APR_STATUS_IS_ENOENT(rv)) { /* Missing an intermediate dir */
         rv = dir_make_parent(path, perm, pool);
 
-        if (rv == APR_SUCCESS) {
-            rv = apr_dir_make (path, perm, pool); /* And complete the path */
+        if (rv == APR_SUCCESS || APR_STATUS_IS_EEXIST(rv)) {
+            rv = apr_dir_make(path, perm, pool); /* And complete the path */
         }
     }
 
@@ -330,28 +330,35 @@ APR_DECLARE(apr_status_t) apr_dir_make_recursive(const char *path,
                                                  apr_pool_t *pool)
 {
     apr_status_t rv = 0;
-    
+
     rv = apr_dir_make (path, perm, pool); /* Try to make PATH right out */
-    
+
     if (APR_STATUS_IS_ENOENT(rv)) { /* Missing an intermediate dir */
         char *dir;
-        
+
         rv = apr_filepath_merge(&dir, "", path, APR_FILEPATH_NATIVE, pool);
 
-        if (rv == APR_SUCCESS)
-            rv = dir_make_parent(dir, perm, pool); /* Make intermediate dirs */
-        
-        if (rv == APR_SUCCESS)
+        if (rv != APR_SUCCESS)
+            return rv;
+
+        rv = dir_make_parent(dir, perm, pool); /* Make intermediate dirs */
+
+        if (rv == APR_SUCCESS || APR_STATUS_IS_EEXIST(rv)) {
             rv = apr_dir_make (dir, perm, pool);   /* And complete the path */
-    }
 
-    /*
-     * It's OK if PATH exists. Timing issues can lead to the second
-     * apr_dir_make being called on existing dir, therefore this check
-     * has to come last.
-     */
-    if (APR_STATUS_IS_EEXIST(rv))
-        return APR_SUCCESS;
+            if (APR_STATUS_IS_EEXIST(rv)) {
+                rv = APR_SUCCESS; /* Timing issue; see comment below */
+            }
+        }
+    }
+    else if (APR_STATUS_IS_EEXIST(rv)) {
+        /*
+         * It's OK if PATH exists. Timing issues can lead to the
+         * second apr_dir_make being called on existing dir, therefore
+         * this check has to come last.
+         */
+        rv = APR_SUCCESS;
+    }
 
     return rv;
 }
diff --git a/test/testdir.c b/test/testdir.c
index 417a1d7b2..65ac67a2b 100644
--- a/test/testdir.c
+++ b/test/testdir.c
@@ -22,6 +22,7 @@
 #include "apr_errno.h"
 #include "apr_general.h"
 #include "apr_lib.h"
+#include "apr_thread_proc.h"
 #include "testutil.h"
 
 static void test_mkdir(abts_case *tc, void *data)
@@ -59,6 +60,60 @@ static void test_mkdir_recurs(abts_case *tc, void *data)
     ABTS_INT_EQUAL(tc, APR_DIR, finfo.filetype);
 }
 
+static void *APR_THREAD_FUNC thread_mkdir_func(apr_thread_t *thd, void *data)
+{
+    abts_case *tc = data;
+    apr_status_t s1, s2, s3, s4, s5;
+
+    s1 = apr_dir_make_recursive("data/prll/one/thwo/three",
+                                APR_FPROT_UREAD | APR_FPROT_UWRITE | APR_FPROT_UEXECUTE,
+                                p);
+    s2 = apr_dir_make_recursive("data/prll/four/five/six/seven/eight",
+                                APR_FPROT_UREAD | APR_FPROT_UWRITE | APR_FPROT_UEXECUTE,
+                                p);
+    s3 = apr_dir_make_recursive("data/prll/nine/ten",
+                                APR_FPROT_UREAD | APR_FPROT_UWRITE | APR_FPROT_UEXECUTE,
+                                p);
+    s4 = apr_dir_make_recursive("data/prll/11/12/13/14/15/16/17/18/19/20",
+                                APR_FPROT_UREAD | APR_FPROT_UWRITE | APR_FPROT_UEXECUTE,
+                                p);
+    s5 = apr_dir_make_recursive("data/fortytwo",
+                                APR_FPROT_UREAD | APR_FPROT_UWRITE | APR_FPROT_UEXECUTE,
+                                p);
+
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s1);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s2);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s3);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s4);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s5);
+    return NULL;
+}
+
+static void test_mkdir_recurs_parallel(abts_case *tc, void *data)
+{
+    apr_thread_t *t1, *t2, *t3, *t4;
+    apr_status_t s1, s2, s3, s4;
+
+    s1 = apr_thread_create(&t1, NULL, thread_mkdir_func, tc, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s1);
+    s2 = apr_thread_create(&t2, NULL, thread_mkdir_func, tc, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s2);
+    s3 = apr_thread_create(&t3, NULL, thread_mkdir_func, tc, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s3);
+    s4 = apr_thread_create(&t4, NULL, thread_mkdir_func, tc, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s4);
+
+    apr_thread_join(&s1, t1);
+    apr_thread_join(&s2, t2);
+    apr_thread_join(&s3, t3);
+    apr_thread_join(&s4, t4);
+
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s1);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s2);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s3);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, s4);
+}
+
 static void test_remove(abts_case *tc, void *data)
 {
     apr_status_t rv;
@@ -91,6 +146,72 @@ static void test_removeall(abts_case *tc, void *data)
 
     rv = apr_dir_remove("data/one", p);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/one/thwo/three", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/one/thwo", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/one", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/four/five/six/seven/eight", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/four/five/six/seven", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/four/five/six", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/four/five", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/four", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/nine/ten", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/nine", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14/15/16/17/18/19/20", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14/15/16/17/18/19", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14/15/16/17/18", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14/15/16/17", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14/15/16", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14/15", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13/14", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12/13", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11/12", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll/11", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/prll", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    rv = apr_dir_remove("data/fortytwo", p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
 }
 
 static void test_remove_notthere(abts_case *tc, void *data)
@@ -245,6 +366,7 @@ abts_suite *testdir(abts_suite *suite)
 
     abts_run_test(suite, test_mkdir, NULL);
     abts_run_test(suite, test_mkdir_recurs, NULL);
+    abts_run_test(suite, test_mkdir_recurs_parallel, NULL);
     abts_run_test(suite, test_remove, NULL);
     abts_run_test(suite, test_removeall_fail, NULL);
     abts_run_test(suite, test_removeall, NULL);