From 234c1d9ff4b5e2f04ee8a4fd1cde9c78b9aa3cd8 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Tue, 31 Dec 2013 11:36:53 +0000 Subject: [PATCH] weed out race-condition in mkdirs calls inspired by PR 55290 and Matthias Bhend's suggestion git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@1554403 13f79535-47bb-0310-9956-ffa450edef68 --- CONTRIBUTORS | 1 + WHATSNEW | 4 ++++ contributors.xml | 24 ++++++++++++------- .../org/apache/tools/ant/taskdefs/Copy.java | 2 +- .../org/apache/tools/ant/taskdefs/Move.java | 2 +- .../org/apache/tools/ant/taskdefs/Tar.java | 3 ++- .../tools/ant/taskdefs/XSLTProcess.java | 2 +- .../org/apache/tools/ant/taskdefs/Zip.java | 3 ++- .../tools/ant/taskdefs/compilers/Gcj.java | 3 ++- .../ant/taskdefs/optional/Native2Ascii.java | 3 ++- .../ant/taskdefs/optional/image/Image.java | 3 ++- .../ant/taskdefs/optional/pvcs/Pvcs.java | 2 +- .../tools/ant/taskdefs/optional/sos/SOS.java | 2 +- .../org/apache/tools/ant/util/FileUtils.java | 3 ++- .../apache/tools/ant/util/ResourceUtils.java | 2 +- 15 files changed, 39 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index b2a74bbdb..009bb30bc 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -239,6 +239,7 @@ Matthew Hawthorne Matthew Inger Matthew Kuperus Heun Matthew Watson +Matthias Bhend Michael Bayne Michael Clarke Michael Davey diff --git a/WHATSNEW b/WHATSNEW index 72b4d4e44..370ce7956 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -17,6 +17,10 @@ Fixed bugs: different resource types. Bugzilla Report 55097 + * several calls to File#mkdirs could fall victim to a race condition + where another thread already created the same directory. + Bugzilla Report 55290 + Other changes: -------------- diff --git a/contributors.xml b/contributors.xml index 7853a41af..e3a1bcee2 100644 --- a/contributors.xml +++ b/contributors.xml @@ -924,14 +924,6 @@ Martin von Gagern - - Mathieu - Champlon - - - Mathieu - Peltier - Matt Albrecht @@ -960,6 +952,22 @@ Matt Small + + Matt + Wildig + + + Mathieu + Champlon + + + Mathieu + Peltier + + + Matthias + Bhend + Matthew Hawthorne diff --git a/src/main/org/apache/tools/ant/taskdefs/Copy.java b/src/main/org/apache/tools/ant/taskdefs/Copy.java index da9578739..2d72ed69f 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Copy.java +++ b/src/main/org/apache/tools/ant/taskdefs/Copy.java @@ -915,7 +915,7 @@ public class Copy extends Task { for (int i = 0; i < dirs.length; i++) { File d = new File(dirs[i]); if (!d.exists()) { - if (!d.mkdirs()) { + if (!(d.mkdirs() || d.isDirectory())) { log("Unable to create directory " + d.getAbsolutePath(), Project.MSG_ERR); } else { diff --git a/src/main/org/apache/tools/ant/taskdefs/Move.java b/src/main/org/apache/tools/ant/taskdefs/Move.java index 99404b920..4c27bac5a 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Move.java +++ b/src/main/org/apache/tools/ant/taskdefs/Move.java @@ -178,7 +178,7 @@ public class Move extends Copy { } File d = new File(toDirNames[i]); if (!d.exists()) { - if (!d.mkdirs()) { + if (!(d.mkdirs() || d.exists())) { log("Unable to create directory " + d.getAbsolutePath(), Project.MSG_ERR); } else { diff --git a/src/main/org/apache/tools/ant/taskdefs/Tar.java b/src/main/org/apache/tools/ant/taskdefs/Tar.java index 000f2f415..a0230dcd4 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Tar.java +++ b/src/main/org/apache/tools/ant/taskdefs/Tar.java @@ -281,7 +281,8 @@ public class Tar extends MatchingTask { } File parent = tarFile.getParentFile(); - if (parent != null && !parent.isDirectory() && !parent.mkdirs()) { + if (parent != null && !parent.isDirectory() + && !(parent.mkdirs() || parent.isDirectory())) { throw new BuildException("Failed to create missing parent" + " directory for " + tarFile); } diff --git a/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java b/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java index e602b9b56..94271f54e 100644 --- a/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java +++ b/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java @@ -896,7 +896,7 @@ public class XSLTProcess extends MatchingTask implements XSLTLogger { private void ensureDirectoryFor(File targetFile) throws BuildException { File directory = targetFile.getParentFile(); if (!directory.exists()) { - if (!directory.mkdirs()) { + if (!(directory.mkdirs() || directory.isDirectory())) { handleError("Unable to create directory: " + directory.getAbsolutePath()); } diff --git a/src/main/org/apache/tools/ant/taskdefs/Zip.java b/src/main/org/apache/tools/ant/taskdefs/Zip.java index 4bd32c7e9..3d05349a4 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Zip.java +++ b/src/main/org/apache/tools/ant/taskdefs/Zip.java @@ -645,7 +645,8 @@ public class Zip extends MatchingTask { } File parent = zipFile.getParentFile(); - if (parent != null && !parent.isDirectory() && !parent.mkdirs()) { + if (parent != null && !parent.isDirectory() + && !(parent.mkdirs() || parent.isDirectory())) { throw new BuildException("Failed to create missing parent" + " directory for " + zipFile); } diff --git a/src/main/org/apache/tools/ant/taskdefs/compilers/Gcj.java b/src/main/org/apache/tools/ant/taskdefs/compilers/Gcj.java index 889d66712..3167cc244 100644 --- a/src/main/org/apache/tools/ant/taskdefs/compilers/Gcj.java +++ b/src/main/org/apache/tools/ant/taskdefs/compilers/Gcj.java @@ -87,7 +87,8 @@ public class Gcj extends DefaultCompilerAdapter { cmd.createArgument().setValue("-d"); cmd.createArgument().setFile(destDir); - if (!destDir.exists() && !destDir.mkdirs()) { + if (!destDir.exists() + && !(destDir.mkdirs() || destDir.isDirectory())) { throw new BuildException("Can't make output directories. " + "Maybe permission is wrong. "); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/Native2Ascii.java b/src/main/org/apache/tools/ant/taskdefs/optional/Native2Ascii.java index 2ff92f0ff..49b086acf 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/Native2Ascii.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/Native2Ascii.java @@ -279,7 +279,8 @@ public class Native2Ascii extends MatchingTask { if (parentName != null) { File parentFile = new File(parentName); - if ((!parentFile.exists()) && (!parentFile.mkdirs())) { + if (!parentFile.exists() + && !(parentFile.mkdirs() || parentFile.isDirectory())) { throw new BuildException("cannot create parent directory " + parentName); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/image/Image.java b/src/main/org/apache/tools/ant/taskdefs/optional/image/Image.java index 8b5a2f919..1567c6760 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/image/Image.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/image/Image.java @@ -305,7 +305,8 @@ public class Image extends MatchingTask { } File dstParent = newFile.getParentFile(); - if (!dstParent.isDirectory() && !dstParent.mkdirs()){ + if (!dstParent.isDirectory() + && !(dstParent.mkdirs() || dstParent.isDirectory())) { throw new BuildException("Failed to create parent directory " + dstParent); } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/pvcs/Pvcs.java b/src/main/org/apache/tools/ant/taskdefs/optional/pvcs/Pvcs.java index c42f0348f..3e105639c 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/pvcs/Pvcs.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/pvcs/Pvcs.java @@ -319,7 +319,7 @@ public class Pvcs extends org.apache.tools.ant.Task { if (!dir.exists()) { log("Creating " + dir.getAbsolutePath(), Project.MSG_VERBOSE); - if (dir.mkdirs()) { + if (dir.mkdirs() || dir.isDirectory()) { log("Created " + dir.getAbsolutePath(), Project.MSG_INFO); } else { diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/sos/SOS.java b/src/main/org/apache/tools/ant/taskdefs/optional/sos/SOS.java index 8c1992a59..80bb1041d 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/sos/SOS.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/sos/SOS.java @@ -362,7 +362,7 @@ public abstract class SOS extends Task implements SOSCmd { // make sure localDir exists, create it if it doesn't File dir = getProject().resolveFile(localPath); if (!dir.exists()) { - boolean done = dir.mkdirs(); + boolean done = dir.mkdirs() || dir.isDirectory(); if (!done) { String msg = "Directory " + localPath + " creation was not " + "successful for an unknown reason"; diff --git a/src/main/org/apache/tools/ant/util/FileUtils.java b/src/main/org/apache/tools/ant/util/FileUtils.java index caaa82b61..01030107f 100644 --- a/src/main/org/apache/tools/ant/util/FileUtils.java +++ b/src/main/org/apache/tools/ant/util/FileUtils.java @@ -1308,7 +1308,8 @@ public class FileUtils { throw new IOException("Failed to delete " + to + " while trying to rename " + from); } File parent = to.getParentFile(); - if (parent != null && !parent.exists() && !parent.mkdirs()) { + if (parent != null && !parent.isDirectory() + && !(parent.mkdirs() || parent.isDirectory())) { throw new IOException("Failed to create directory " + parent + " while trying to rename " + from); } diff --git a/src/main/org/apache/tools/ant/util/ResourceUtils.java b/src/main/org/apache/tools/ant/util/ResourceUtils.java index 7b6bc2557..e4562db55 100644 --- a/src/main/org/apache/tools/ant/util/ResourceUtils.java +++ b/src/main/org/apache/tools/ant/util/ResourceUtils.java @@ -513,7 +513,7 @@ public class ResourceUtils { File parent = destFile.getParentFile(); if (parent != null && !parent.isDirectory() - && !destFile.getParentFile().mkdirs()) { + && !(parent.mkdirs() || parent.isDirectory())) { throw new IOException("failed to create the parent directory" + " for " + destFile); }