Skip to content

Commit

Permalink
Return success from FileManager recursive directory creation even i…
Browse files Browse the repository at this point in the history
…f 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).
  • Loading branch information
fumoboy007 committed Feb 27, 2020
1 parent d30c63b commit 1a1402e
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
12 changes: 10 additions & 2 deletions Sources/Foundation/FileManager+POSIX.swift
Expand Up @@ -346,8 +346,16 @@ extension FileManager {
try createDirectory(atPath: parent, withIntermediateDirectories: true, attributes: attributes)
}
if mkdir(pathFsRep, S_IRWXU | S_IRWXG | S_IRWXO) != 0 {
throw _NSErrorWithErrno(errno, reading: false, path: path)
} else if let attr = attributes {
let posixError = errno
if posixError == EEXIST && fileExists(atPath: path, isDirectory: &isDir) && isDir.boolValue {
// Continue; if there is an existing file and it is a directory, that is still a success.
// There can be an existing file if another thread or process concurrently creates the
// same file.
} else {
throw _NSErrorWithErrno(posixError, reading: false, path: path)
}
}
if let attr = attributes {
try self.setAttributes(attr, ofItemAtPath: path)
}
} else if isDir.boolValue {
Expand Down
37 changes: 37 additions & 0 deletions Tests/Foundation/Tests/TestFileManager.swift
Expand Up @@ -15,6 +15,8 @@
#endif
#endif

import Dispatch

class TestFileManager : XCTestCase {
#if os(Windows)
let pathSep = "\\"
Expand Down Expand Up @@ -1755,6 +1757,40 @@ VIDEOS=StopgapVideos
}
#endif
}

/**
Tests that we can get an item replacement directory concurrently.
- Bug: [SR-12272](https://bugs.swift.org/browse/SR-12272)
*/
func test_concurrentGetItemReplacementDirectory() throws {
let dispatchGroup = DispatchGroup()

let operationCount = 10
var errors = [Error?](repeating: nil, count: operationCount)

for operationIndex in 0..<operationCount {
DispatchQueue.global().async(group: dispatchGroup) {
do {
_ = try FileManager.default.url(for: .itemReplacementDirectory,
in: .userDomainMask,
appropriateFor: URL(fileURLWithPath: NSTemporaryDirectory(),
isDirectory: true),
create: true)
} catch {
errors[operationIndex] = error
}
}
}

dispatchGroup.wait()

for error in errors {
if let error = error {
throw error
}
}
}

// -----

Expand Down Expand Up @@ -1812,6 +1848,7 @@ VIDEOS=StopgapVideos
("test_contentsEqual", test_contentsEqual),
/* ⚠️ */ ("test_replacement", testExpectedToFail(test_replacement,
/* ⚠️ */ "<https://bugs.swift.org/browse/SR-10819> Re-enable Foundation test TestFileManager.test_replacement")),
("test_concurrentGetItemReplacementDirectory", test_concurrentGetItemReplacementDirectory),
]

#if !DEPLOYMENT_RUNTIME_OBJC && NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT && !os(Android)
Expand Down

0 comments on commit 1a1402e

Please sign in to comment.