From 80ea5f9767f77e0878d5cb599d181c4da0a3b00f Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 8 Sep 2021 14:47:07 +0200 Subject: [PATCH 1/4] CreateDirectory: eliminate some syscalls. --- .../src/System/IO/FileSystem.Unix.cs | 156 ++++++++++-------- 1 file changed, 88 insertions(+), 68 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index aab00ededadfe..05696351e2355 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -272,101 +272,121 @@ public static void DeleteFile(string fullPath) public static void CreateDirectory(string fullPath) { - // NOTE: This logic is primarily just carried forward from Win32FileSystem.CreateDirectory. + // The argument is a full path, which means it is an absolute path that + // doesn't contain "//", "/./", and "/../". + Debug.Assert(fullPath.Length > 0); + Debug.Assert(PathInternal.IsDirectorySeparator(fullPath[0])); - int length = fullPath.Length; - - // We need to trim the trailing slash or the code will try to create 2 directories of the same name. - if (length >= 2 && Path.EndsInDirectorySeparator(fullPath)) + string errorString = fullPath; + int result = Interop.Sys.MkDir(fullPath, (int)Interop.Sys.Permissions.Mask); + if (result == 0) { - length--; + // Created directory. + return; } - // For paths that are only // or /// - if (length == 2 && PathInternal.IsDirectorySeparator(fullPath[1])) + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.EEXIST) { - throw new IOException(SR.Format(SR.IO_CannotCreateDirectory, fullPath)); + // Path already exists. Ensure it's a directory. + if (DirectoryExists(fullPath)) + { + return; + } } - - // We can save a bunch of work if the directory we want to create already exists. - if (DirectoryExists(fullPath)) + else if (errorInfo.Error == Interop.Error.ENOENT) { - return; - } + // Some parts of the path don't exist yet. + Debug.Assert(fullPath.Length > 1); - // Attempt to figure out which directories don't exist, and only create the ones we need. - bool somepathexists = false; - List stackDir = new List(); - int lengthRoot = PathInternal.GetRootLength(fullPath); - if (length > lengthRoot) - { + int length = fullPath.Length; + // Trim trailling separator for parent directory lookup. + // The smallest path with trailing separator has length 3, for example '/a/'. + if (length >= 3 && Path.EndsInDirectorySeparator(fullPath)) + { + length--; + } + + // Try create parents bottom to top, track those that don't exist to create them later. + List stackDir = new List(); + stackDir.Add(fullPath); int i = length - 1; - while (i >= lengthRoot && !somepathexists) + do { - if (!DirectoryExists(fullPath.AsSpan(0, i + 1))) // Create only the ones missing + Debug.Assert(!PathInternal.IsDirectorySeparator(fullPath[i])); + + while (!PathInternal.IsDirectorySeparator(fullPath[i])) { - stackDir.Add(fullPath.Substring(0, i + 1)); + i--; } - else + string parentDir = fullPath.Substring(0, i); + + errorString = parentDir; + result = Interop.Sys.MkDir(parentDir, (int)Interop.Sys.Permissions.Mask); + if (result == 0) { - somepathexists = true; + // Created parentDir. + break; } - while (i > lengthRoot && !PathInternal.IsDirectorySeparator(fullPath[i])) + errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.ENOENT) { - i--; + // Some parts of the path don't exist yet. + // We'll try to create its parent on the next iteration. + + // Track this path for creation. + stackDir.Add(parentDir); + } + else if (errorInfo.Error == Interop.Error.EEXIST) + { + // Parent exists. + // If it's not a directory, MkDir will fail when we create a child directory. + result = 0; + break; + } + else + { + // Fail. + break; } i--; - } - } - - int count = stackDir.Count; - if (count == 0 && !somepathexists) - { - ReadOnlySpan root = Path.GetPathRoot(fullPath.AsSpan()); - if (!DirectoryExists(root)) - { - throw Interop.GetExceptionForIoErrno(Interop.Error.ENOENT.Info(), fullPath, isDirectory: true); - } - return; - } - - // Create all the directories - int result = 0; - Interop.ErrorInfo firstError = default(Interop.ErrorInfo); - string errorString = fullPath; - for (int i = stackDir.Count - 1; i >= 0; i--) - { - string name = stackDir[i]; + } while (i > 0); - // The mkdir command uses 0777 by default (it'll be AND'd with the process umask internally). - // We do the same. - result = Interop.Sys.MkDir(name, (int)Interop.Sys.Permissions.Mask); - if (result < 0 && firstError.Error == 0) + if (result == 0) { - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - - // While we tried to avoid creating directories that don't - // exist above, there are a few cases that can fail, e.g. - // a race condition where another process or thread creates - // the directory first, or there's a file at the location. - if (errorInfo.Error != Interop.Error.EEXIST) + for (i = stackDir.Count - 1; i >= 0; i--) { - firstError = errorInfo; - } - else if (FileExists(name) || (!DirectoryExists(name, out errorInfo) && errorInfo.Error == Interop.Error.EACCES)) - { - // If there's a file in this directory's place, or if we have ERROR_ACCESS_DENIED when checking if the directory already exists throw. - firstError = errorInfo; + string name = stackDir[i]; + errorString = name; + result = Interop.Sys.MkDir(name, (int)Interop.Sys.Permissions.Mask); + if (result < 0) + { + errorInfo = Interop.Sys.GetLastErrorInfo(); + + if (errorInfo.Error == Interop.Error.EEXIST) + { + // Path was created since we last checked. + // Continue, and for the last item, which is fullPath, + // verify it is actually a directory. + if (i == 0 && DirectoryExists(name)) + { + return; + } + } + else + { + break; + } + } } } } - // Only throw an exception if creating the exact directory we wanted failed to work correctly. - if (result < 0 && firstError.Error != 0) + if (result < 0) { - throw Interop.GetExceptionForIoErrno(firstError, errorString, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, errorString, isDirectory: true); } } From 15434976134ab468bdf332ae4ad841e42f3fac73 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 8 Sep 2021 22:04:15 +0200 Subject: [PATCH 2/4] Explicitly handle '/'. --- .../src/System/IO/FileSystem.Unix.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 05696351e2355..54d9fcb00bc95 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -277,6 +277,13 @@ public static void CreateDirectory(string fullPath) Debug.Assert(fullPath.Length > 0); Debug.Assert(PathInternal.IsDirectorySeparator(fullPath[0])); + int length = fullPath.Length; + if (length == 1) + { + // fullPath is '/'. + return; + } + string errorString = fullPath; int result = Interop.Sys.MkDir(fullPath, (int)Interop.Sys.Permissions.Mask); if (result == 0) @@ -297,9 +304,7 @@ public static void CreateDirectory(string fullPath) else if (errorInfo.Error == Interop.Error.ENOENT) { // Some parts of the path don't exist yet. - Debug.Assert(fullPath.Length > 1); - int length = fullPath.Length; // Trim trailling separator for parent directory lookup. // The smallest path with trailing separator has length 3, for example '/a/'. if (length >= 3 && Path.EndsInDirectorySeparator(fullPath)) From 7b08a5fa8f58abab3574813ebff8adbc33ed3f0a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 8 Sep 2021 23:42:02 +0200 Subject: [PATCH 3/4] cleanup --- .../src/System/IO/FileSystem.Unix.cs | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 54d9fcb00bc95..0acee87fcf2b4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -277,15 +277,14 @@ public static void CreateDirectory(string fullPath) Debug.Assert(fullPath.Length > 0); Debug.Assert(PathInternal.IsDirectorySeparator(fullPath[0])); - int length = fullPath.Length; - if (length == 1) + if (fullPath.Length == 1) { // fullPath is '/'. return; } - string errorString = fullPath; - int result = Interop.Sys.MkDir(fullPath, (int)Interop.Sys.Permissions.Mask); + string mkdirPath = fullPath; + int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); if (result == 0) { // Created directory. @@ -305,32 +304,34 @@ public static void CreateDirectory(string fullPath) { // Some parts of the path don't exist yet. - // Trim trailling separator for parent directory lookup. - // The smallest path with trailing separator has length 3, for example '/a/'. - if (length >= 3 && Path.EndsInDirectorySeparator(fullPath)) - { - length--; - } + // Try create parents bottom to top and track those that could not + // be created due to missing parents. Then create them top to bottom. - // Try create parents bottom to top, track those that don't exist to create them later. List stackDir = new List(); + stackDir.Add(fullPath); - int i = length - 1; + + int i = fullPath.Length - 1; + // Trim trailing separator. + if (PathInternal.IsDirectorySeparator(fullPath[i])) + { + i--; + } do { + // Find the end of the parent directory. Debug.Assert(!PathInternal.IsDirectorySeparator(fullPath[i])); - while (!PathInternal.IsDirectorySeparator(fullPath[i])) { i--; } - string parentDir = fullPath.Substring(0, i); - errorString = parentDir; - result = Interop.Sys.MkDir(parentDir, (int)Interop.Sys.Permissions.Mask); + // Try create it. + mkdirPath = fullPath.Substring(0, i); + result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); if (result == 0) { - // Created parentDir. + // Created parent. break; } @@ -340,13 +341,13 @@ public static void CreateDirectory(string fullPath) // Some parts of the path don't exist yet. // We'll try to create its parent on the next iteration. - // Track this path for creation. - stackDir.Add(parentDir); + // Track this path for later creation. + stackDir.Add(mkdirPath); } else if (errorInfo.Error == Interop.Error.EEXIST) { // Parent exists. - // If it's not a directory, MkDir will fail when we create a child directory. + // If it is not a directory, MkDir will fail when we create a child directory. result = 0; break; } @@ -360,12 +361,11 @@ public static void CreateDirectory(string fullPath) if (result == 0) { + // Create directories that had missing parents. for (i = stackDir.Count - 1; i >= 0; i--) { - string name = stackDir[i]; - - errorString = name; - result = Interop.Sys.MkDir(name, (int)Interop.Sys.Permissions.Mask); + mkdirPath = stackDir[i]; + result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); if (result < 0) { errorInfo = Interop.Sys.GetLastErrorInfo(); @@ -375,13 +375,14 @@ public static void CreateDirectory(string fullPath) // Path was created since we last checked. // Continue, and for the last item, which is fullPath, // verify it is actually a directory. - if (i == 0 && DirectoryExists(name)) + if (i == 0 && DirectoryExists(mkdirPath)) { return; } } else { + // Fail. break; } } @@ -391,7 +392,7 @@ public static void CreateDirectory(string fullPath) if (result < 0) { - throw Interop.GetExceptionForIoErrno(errorInfo, errorString, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath, isDirectory: true); } } From 0a03e359f5aaa6e189dcdefe19049dcb55907881 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 18 Nov 2021 06:56:38 +0100 Subject: [PATCH 4/4] PR feedback --- .../src/System/IO/FileSystem.Unix.cs | 162 ++++++++---------- 1 file changed, 76 insertions(+), 86 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 0acee87fcf2b4..3f3c59d3bff78 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -279,120 +279,110 @@ public static void CreateDirectory(string fullPath) if (fullPath.Length == 1) { - // fullPath is '/'. - return; + return; // fullPath is '/'. } - string mkdirPath = fullPath; - int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); + int result = Interop.Sys.MkDir(fullPath, (int)Interop.Sys.Permissions.Mask); if (result == 0) { - // Created directory. - return; + return; // Created directory. } Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.EEXIST) + if (errorInfo.Error == Interop.Error.EEXIST && DirectoryExists(fullPath)) { - // Path already exists. Ensure it's a directory. - if (DirectoryExists(fullPath)) - { - return; - } + return; // Path already exists and it's a directory. } else if (errorInfo.Error == Interop.Error.ENOENT) { // Some parts of the path don't exist yet. + CreateParentsAndDirectory(fullPath); + } + else + { + throw Interop.GetExceptionForIoErrno(errorInfo, fullPath, isDirectory: true); + } + } - // Try create parents bottom to top and track those that could not - // be created due to missing parents. Then create them top to bottom. - - List stackDir = new List(); + public static void CreateParentsAndDirectory(string fullPath) + { + // Try create parents bottom to top and track those that could not + // be created due to missing parents. Then create them top to bottom. + List stackDir = new List(); - stackDir.Add(fullPath); + stackDir.Add(fullPath); - int i = fullPath.Length - 1; - // Trim trailing separator. - if (PathInternal.IsDirectorySeparator(fullPath[i])) + int i = fullPath.Length - 1; + // Trim trailing separator. + if (PathInternal.IsDirectorySeparator(fullPath[i])) + { + i--; + } + do + { + // Find the end of the parent directory. + Debug.Assert(!PathInternal.IsDirectorySeparator(fullPath[i])); + while (!PathInternal.IsDirectorySeparator(fullPath[i])) { i--; } - do - { - // Find the end of the parent directory. - Debug.Assert(!PathInternal.IsDirectorySeparator(fullPath[i])); - while (!PathInternal.IsDirectorySeparator(fullPath[i])) - { - i--; - } - // Try create it. - mkdirPath = fullPath.Substring(0, i); - result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); - if (result == 0) - { - // Created parent. - break; - } + // Try create it. + string mkdirPath = fullPath.Substring(0, i); + int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); + if (result == 0) + { + // Created parent. + break; + } - errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.ENOENT) - { - // Some parts of the path don't exist yet. - // We'll try to create its parent on the next iteration. + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.ENOENT) + { + // Some parts of the path don't exist yet. + // We'll try to create its parent on the next iteration. - // Track this path for later creation. - stackDir.Add(mkdirPath); - } - else if (errorInfo.Error == Interop.Error.EEXIST) - { - // Parent exists. - // If it is not a directory, MkDir will fail when we create a child directory. - result = 0; - break; - } - else - { - // Fail. - break; - } - i--; - } while (i > 0); + // Track this path for later creation. + stackDir.Add(mkdirPath); + } + else if (errorInfo.Error == Interop.Error.EEXIST) + { + // Parent exists. + // If it is not a directory, MkDir will fail when we create a child directory. + break; + } + else + { + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath, isDirectory: true); + } + i--; + } while (i > 0); - if (result == 0) + // Create directories that had missing parents. + for (i = stackDir.Count - 1; i >= 0; i--) + { + string mkdirPath = stackDir[i]; + int result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); + if (result < 0) { - // Create directories that had missing parents. - for (i = stackDir.Count - 1; i >= 0; i--) + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.EEXIST) { - mkdirPath = stackDir[i]; - result = Interop.Sys.MkDir(mkdirPath, (int)Interop.Sys.Permissions.Mask); - if (result < 0) + // Path was created since we last checked. + // Continue, and for the last item, which is fullPath, + // verify it is actually a directory. + if (i != 0) { - errorInfo = Interop.Sys.GetLastErrorInfo(); - - if (errorInfo.Error == Interop.Error.EEXIST) - { - // Path was created since we last checked. - // Continue, and for the last item, which is fullPath, - // verify it is actually a directory. - if (i == 0 && DirectoryExists(mkdirPath)) - { - return; - } - } - else - { - // Fail. - break; - } + continue; + } + if (DirectoryExists(mkdirPath)) + { + return; } } - } - } - if (result < 0) - { - throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, mkdirPath, isDirectory: true); + } } }