Skip to content

Commit b6eea02

Browse files
committed
Return success from FileManager recursive directory creation even if a directory was created by another thread concurrently.
If another thread creates one of the same directories in the target path, the `!fileExists` check may pass while the `mkdir` call fails with `EEXIST`. However, if the existing file is a directory, then that should still be a success. Fixes [SR-12272](https://bugs.swift.org/browse/SR-12272).
1 parent 2fc16a2 commit b6eea02

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

Sources/Foundation/FileManager+POSIX.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,8 +346,16 @@ extension FileManager {
346346
try createDirectory(atPath: parent, withIntermediateDirectories: true, attributes: attributes)
347347
}
348348
if mkdir(pathFsRep, S_IRWXU | S_IRWXG | S_IRWXO) != 0 {
349-
throw _NSErrorWithErrno(errno, reading: false, path: path)
350-
} else if let attr = attributes {
349+
let posixError = errno
350+
if posixError == EEXIST && fileExists(atPath: path, isDirectory: &isDir) && isDir.boolValue {
351+
// Continue; if there is an existing file and it is a directory, that is still a success.
352+
// There can be an existing file if another thread or process concurrently creates the
353+
// same file.
354+
} else {
355+
throw _NSErrorWithErrno(posixError, reading: false, path: path)
356+
}
357+
}
358+
if let attr = attributes {
351359
try self.setAttributes(attr, ofItemAtPath: path)
352360
}
353361
} else if isDir.boolValue {

Tests/Foundation/Tests/TestFileManager.swift

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#endif
1616
#endif
1717

18+
import Dispatch
19+
1820
class TestFileManager : XCTestCase {
1921
#if os(Windows)
2022
let pathSep = "\\"
@@ -1870,6 +1872,49 @@ VIDEOS=StopgapVideos
18701872
try checkPath(path: path)
18711873
}
18721874
}
1875+
1876+
/**
1877+
Tests that we can get an item replacement directory concurrently.
1878+
1879+
- Bug: [SR-12272](https://bugs.swift.org/browse/SR-12272)
1880+
*/
1881+
func test_concurrentGetItemReplacementDirectory() throws {
1882+
let fileManager = FileManager.default
1883+
1884+
let operationCount = 10
1885+
1886+
var directoryURLs = [URL?](repeating: nil, count: operationCount)
1887+
var errors = [Error?](repeating: nil, count: operationCount)
1888+
1889+
let dispatchGroup = DispatchGroup()
1890+
for operationIndex in 0..<operationCount {
1891+
DispatchQueue.global().async(group: dispatchGroup) {
1892+
do {
1893+
let directory = try fileManager.url(for: .itemReplacementDirectory,
1894+
in: .userDomainMask,
1895+
appropriateFor: URL(fileURLWithPath: NSTemporaryDirectory(),
1896+
isDirectory: true),
1897+
create: true)
1898+
directoryURLs[operationIndex] = directory
1899+
} catch {
1900+
errors[operationIndex] = error
1901+
}
1902+
}
1903+
}
1904+
dispatchGroup.wait()
1905+
1906+
for directoryURL in directoryURLs {
1907+
if let directoryURL = directoryURL {
1908+
try? fileManager.removeItem(at: directoryURL)
1909+
}
1910+
}
1911+
1912+
for error in errors {
1913+
if let error = error {
1914+
XCTFail("One of the concurrent calls to get the item replacement directory failed: \(error)")
1915+
}
1916+
}
1917+
}
18731918

18741919
// -----
18751920

@@ -1928,6 +1973,7 @@ VIDEOS=StopgapVideos
19281973
("test_contentsEqual", test_contentsEqual),
19291974
/* ⚠️ */ ("test_replacement", testExpectedToFail(test_replacement,
19301975
/* ⚠️ */ "<https://bugs.swift.org/browse/SR-10819> Re-enable Foundation test TestFileManager.test_replacement")),
1976+
("test_concurrentGetItemReplacementDirectory", test_concurrentGetItemReplacementDirectory),
19311977
]
19321978

19331979
#if !DEPLOYMENT_RUNTIME_OBJC && NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT && !os(Android)

0 commit comments

Comments
 (0)