From 34a895f8ab602c141c33ca940812d9157faf3030 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Fri, 2 Jun 2023 15:43:46 +0100 Subject: [PATCH 1/2] Add `async` overloads of `withLock` for FS I/O When converting parts of the SwiftPM codebase from callbacks to `async`/`await` I've stumbled upon uses of file system locking that have to work across an async closure call. This is an additive change and has no impact on the existing blocking non-async uses of `FileLock.withLock`. --- Sources/TSCBasic/FileSystem.swift | 27 +++++++++++--- Sources/TSCBasic/Lock.swift | 62 ++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/Sources/TSCBasic/FileSystem.swift b/Sources/TSCBasic/FileSystem.swift index 023e6e43..bebc0475 100644 --- a/Sources/TSCBasic/FileSystem.swift +++ b/Sources/TSCBasic/FileSystem.swift @@ -288,6 +288,9 @@ public protocol FileSystem: Sendable { /// Execute the given block while holding the lock. func withLock(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T + + /// Execute the given block while holding the lock. + func withLock(on path: AbsolutePath, type: FileLock.LockType, _ body: () async throws -> T) async throws -> T } /// Convenience implementations (default arguments aren't permitted in protocol @@ -336,6 +339,10 @@ public extension FileSystem { throw FileSystemError(.unsupported, path) } + func withLock(on path: AbsolutePath, type: FileLock.LockType, _ body: () async throws -> T) async throws -> T { + throw FileSystemError(.unsupported, path) + } + func hasQuarantineAttribute(_ path: AbsolutePath) -> Bool { false } func hasAttribute(_ name: FileSystemAttribute, _ path: AbsolutePath) -> Bool { false } @@ -601,12 +608,20 @@ private struct LocalFileSystem: FileSystem { func withLock(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T { try FileLock.withLock(fileToLock: path, type: type, body: body) } + + func withLock( + on path: AbsolutePath, + type: FileLock.LockType = .exclusive, + _ body: () async throws -> T + ) async throws -> T { + try await FileLock.withLock(fileToLock: path, type: type, body: body) + } } /// Concrete FileSystem implementation which simulates an empty disk. public final class InMemoryFileSystem: FileSystem { /// Private internal representation of a file system node. - /// Not threadsafe. + /// Not thread-safe. private class Node { /// The actual node data. let contents: NodeContents @@ -622,7 +637,7 @@ public final class InMemoryFileSystem: FileSystem { } /// Private internal representation the contents of a file system node. - /// Not threadsafe. + /// Not thread-safe. private enum NodeContents { case file(ByteString) case directory(DirectoryContents) @@ -642,7 +657,7 @@ public final class InMemoryFileSystem: FileSystem { } /// Private internal representation the contents of a directory. - /// Not threadsafe. + /// Not thread-safe. private final class DirectoryContents { var entries: [String: Node] @@ -697,7 +712,7 @@ public final class InMemoryFileSystem: FileSystem { } /// Private function to look up the node corresponding to a path. - /// Not threadsafe. + /// Not thread-safe. private func getNode(_ path: AbsolutePath, followSymlink: Bool = true) throws -> Node? { func getNodeInternal(_ path: AbsolutePath) throws -> Node? { // If this is the root node, return it. @@ -841,7 +856,7 @@ public final class InMemoryFileSystem: FileSystem { } } - /// Not threadsafe. + /// Not thread-safe. private func _createDirectory(_ path: AbsolutePath, recursive: Bool) throws { // Ignore if client passes root. guard !path.isRoot else { @@ -989,7 +1004,7 @@ public final class InMemoryFileSystem: FileSystem { } /// Private implementation of core copying function. - /// Not threadsafe. + /// Not thread-safe. private func _copy(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws { // Get the source node. guard let source = try getNode(sourcePath) else { diff --git a/Sources/TSCBasic/Lock.swift b/Sources/TSCBasic/Lock.swift index 278d1086..432bd2db 100644 --- a/Sources/TSCBasic/Lock.swift +++ b/Sources/TSCBasic/Lock.swift @@ -177,8 +177,20 @@ public final class FileLock { defer { unlock() } return try body() } - - public static func withLock(fileToLock: AbsolutePath, lockFilesDirectory: AbsolutePath? = nil, type: LockType = .exclusive, body: () throws -> T) throws -> T { + + /// Execute the given block while holding the lock. + public func withLock(type: LockType = .exclusive, _ body: () async throws -> T) async throws -> T { + try lock(type: type) + defer { unlock() } + return try await body() + } + + public static func withLock( + fileToLock: AbsolutePath, + lockFilesDirectory: AbsolutePath? = nil, + type: LockType = .exclusive, + body: () throws -> T + ) throws -> T { // unless specified, we use the tempDirectory to store lock files let lockFilesDirectory = try lockFilesDirectory ?? localFileSystem.tempDirectory if !localFileSystem.exists(lockFilesDirectory) { @@ -218,4 +230,50 @@ public final class FileLock { let lock = FileLock(at: lockFilePath) return try lock.withLock(type: type, body) } + + public static func withLock( + fileToLock: AbsolutePath, + lockFilesDirectory: AbsolutePath? = nil, + type: LockType = .exclusive, + body: () async throws -> T + ) async throws -> T { + // unless specified, we use the tempDirectory to store lock files + let lockFilesDirectory = try lockFilesDirectory ?? localFileSystem.tempDirectory + if !localFileSystem.exists(lockFilesDirectory) { + throw FileSystemError(.noEntry, lockFilesDirectory) + } + if !localFileSystem.isDirectory(lockFilesDirectory) { + throw FileSystemError(.notDirectory, lockFilesDirectory) + } + // use the parent path to generate unique filename in temp + var lockFileName = try (resolveSymlinks(fileToLock.parentDirectory) + .appending(component: fileToLock.basename)) + .components.joined(separator: "_") + .replacingOccurrences(of: ":", with: "_") + ".lock" +#if os(Windows) + // NTFS has an ARC limit of 255 codepoints + var lockFileUTF16 = lockFileName.utf16.suffix(255) + while String(lockFileUTF16) == nil { + lockFileUTF16 = lockFileUTF16.dropFirst() + } + lockFileName = String(lockFileUTF16) ?? lockFileName +#else + if lockFileName.hasPrefix(AbsolutePath.root.pathString) { + lockFileName = String(lockFileName.dropFirst(AbsolutePath.root.pathString.count)) + } + // back off until it occupies at most `NAME_MAX` UTF-8 bytes but without splitting scalars + // (we might split clusters but it's not worth the effort to keep them together as long as we get a valid file name) + var lockFileUTF8 = lockFileName.utf8.suffix(Int(NAME_MAX)) + while String(lockFileUTF8) == nil { + // in practice this will only be a few iterations + lockFileUTF8 = lockFileUTF8.dropFirst() + } + // we will never end up with nil since we have ASCII characters at the end + lockFileName = String(lockFileUTF8) ?? lockFileName +#endif + let lockFilePath = lockFilesDirectory.appending(component: lockFileName) + + let lock = FileLock(at: lockFilePath) + return try await lock.withLock(type: type, body) + } } From 91d71c415a003c06aa7890ca4c086136314eadaf Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Fri, 2 Jun 2023 15:48:54 +0100 Subject: [PATCH 2/2] Improve code reuse --- Sources/TSCBasic/Lock.swift | 58 ++++++++++--------------------------- 1 file changed, 15 insertions(+), 43 deletions(-) diff --git a/Sources/TSCBasic/Lock.swift b/Sources/TSCBasic/Lock.swift index 432bd2db..d87186d8 100644 --- a/Sources/TSCBasic/Lock.swift +++ b/Sources/TSCBasic/Lock.swift @@ -185,12 +185,11 @@ public final class FileLock { return try await body() } - public static func withLock( + private static func prepareLock( fileToLock: AbsolutePath, - lockFilesDirectory: AbsolutePath? = nil, - type: LockType = .exclusive, - body: () throws -> T - ) throws -> T { + at lockFilesDirectory: AbsolutePath? = nil, + _ type: LockType = .exclusive + ) throws -> FileLock { // unless specified, we use the tempDirectory to store lock files let lockFilesDirectory = try lockFilesDirectory ?? localFileSystem.tempDirectory if !localFileSystem.exists(lockFilesDirectory) { @@ -227,7 +226,16 @@ public final class FileLock { #endif let lockFilePath = lockFilesDirectory.appending(component: lockFileName) - let lock = FileLock(at: lockFilePath) + return FileLock(at: lockFilePath) + } + + public static func withLock( + fileToLock: AbsolutePath, + lockFilesDirectory: AbsolutePath? = nil, + type: LockType = .exclusive, + body: () throws -> T + ) throws -> T { + let lock = try Self.prepareLock(fileToLock: fileToLock, at: lockFilesDirectory, type) return try lock.withLock(type: type, body) } @@ -237,43 +245,7 @@ public final class FileLock { type: LockType = .exclusive, body: () async throws -> T ) async throws -> T { - // unless specified, we use the tempDirectory to store lock files - let lockFilesDirectory = try lockFilesDirectory ?? localFileSystem.tempDirectory - if !localFileSystem.exists(lockFilesDirectory) { - throw FileSystemError(.noEntry, lockFilesDirectory) - } - if !localFileSystem.isDirectory(lockFilesDirectory) { - throw FileSystemError(.notDirectory, lockFilesDirectory) - } - // use the parent path to generate unique filename in temp - var lockFileName = try (resolveSymlinks(fileToLock.parentDirectory) - .appending(component: fileToLock.basename)) - .components.joined(separator: "_") - .replacingOccurrences(of: ":", with: "_") + ".lock" -#if os(Windows) - // NTFS has an ARC limit of 255 codepoints - var lockFileUTF16 = lockFileName.utf16.suffix(255) - while String(lockFileUTF16) == nil { - lockFileUTF16 = lockFileUTF16.dropFirst() - } - lockFileName = String(lockFileUTF16) ?? lockFileName -#else - if lockFileName.hasPrefix(AbsolutePath.root.pathString) { - lockFileName = String(lockFileName.dropFirst(AbsolutePath.root.pathString.count)) - } - // back off until it occupies at most `NAME_MAX` UTF-8 bytes but without splitting scalars - // (we might split clusters but it's not worth the effort to keep them together as long as we get a valid file name) - var lockFileUTF8 = lockFileName.utf8.suffix(Int(NAME_MAX)) - while String(lockFileUTF8) == nil { - // in practice this will only be a few iterations - lockFileUTF8 = lockFileUTF8.dropFirst() - } - // we will never end up with nil since we have ASCII characters at the end - lockFileName = String(lockFileUTF8) ?? lockFileName -#endif - let lockFilePath = lockFilesDirectory.appending(component: lockFileName) - - let lock = FileLock(at: lockFilePath) + let lock = try Self.prepareLock(fileToLock: fileToLock, at: lockFilesDirectory, type) return try await lock.withLock(type: type, body) } }