Lock Improvements for [SR-12851]

Sources/TSCBasic/CMakeLists.txt

@@ -49,6 +49,7 @@ add_library(TSCBasic
   Thread.swift
   Tuple.swift
   misc.swift)
+
 target_compile_options(TSCBasic PUBLIC
   # Don't use GNU strerror_r on Android.
   "$<$<PLATFORM_ID:Android>:SHELL:-Xcc -U_GNU_SOURCE>"

Sources/TSCBasic/FileSystem.swift

@@ -10,6 +10,7 @@
 
 import TSCLibc
 import Foundation
+import Dispatch
 
 public enum FileSystemError: Swift.Error {
     /// Access to the path is denied.
@@ -200,6 +201,9 @@ public protocol FileSystem: class {
 
     /// Move a file or directory.
     func move(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws
+
+    /// Execute the given block while holding the lock.
+    func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T
 }
 
 /// Convenience implementations (default arguments aren't permitted in protocol
@@ -240,6 +244,10 @@ public extension FileSystem {
     func getFileInfo(_ path: AbsolutePath) throws -> FileInfo {
         throw FileSystemError.unsupported
     }
+
+    func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T {
+        throw FileSystemError.unsupported
+    }
 }
 
 /// Concrete FileSystem implementation which communicates with the local file system.
@@ -450,6 +458,11 @@ private class LocalFileSystem: FileSystem {
         guard !exists(destinationPath) else { throw FileSystemError.alreadyExistsAtDestination }
         try FileManager.default.moveItem(at: sourcePath.asURL, to: destinationPath.asURL)
     }
+
+    func withLock<T>(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T {
+        let lock = FileLock(name: path.basename, cachePath: path.parentDirectory)
+        return try lock.withLock(type: type, body)
+    }
 }
 
 // FIXME: This class does not yet support concurrent mutation safely.
@@ -499,9 +512,21 @@ public class InMemoryFileSystem: FileSystem {
             return contents
         }
     }
+    // Used to ensure that DispatchQueues are releassed when they are no longer in use.
+    private struct WeakReference<Value: AnyObject> {
+        weak var reference: Value?
+
+        init(_ value: Value?) {
+            self.reference = value
+        }
+    }
 
     /// The root filesytem.
     private var root: Node
+    /// A map that keeps weak references to all locked files.
+    private var lockFiles = Dictionary<AbsolutePath, WeakReference<DispatchQueue>>()
+    /// Used to access lockFiles in a thread safe manner.
+    private let lockFilesLock = Lock()
 
     public init() {
         root = Node(.directory(DirectoryContents()))
@@ -760,6 +785,21 @@ public class InMemoryFileSystem: FileSystem {
 
         contents.entries[sourcePath.basename] = nil
     }
+
+    public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T {
+
+        let fileQueue: DispatchQueue = lockFilesLock.withLock {
+            if let queueReference = lockFiles[path], let queue = queueReference.reference {
+                return queue
+            } else {
+                let queue = DispatchQueue(label: "org.swift.swiftpm.in-memory-file-system.file-queue", attributes: .concurrent)
+                lockFiles[path] = WeakReference(queue)
+                return queue
+            }
+        }
+
+        return try fileQueue.sync(flags: type == .exclusive ? .barrier : .init() , execute: body)
+    }
 }
 
 /// A rerooted view on an existing FileSystem.
@@ -864,6 +904,10 @@ public class RerootedFileSystemView: FileSystem {
     public func move(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws {
         try underlyingFileSystem.move(from: formUnderlyingPath(sourcePath), to: formUnderlyingPath(sourcePath))
     }
+
+    public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T {
+        return try underlyingFileSystem.withLock(on: formUnderlyingPath(path), type: type, body)
+    }
 }
 
 /// Public access to the local FS proxy.

Sources/TSCBasic/Lock.swift

@@ -19,10 +19,18 @@ public struct Lock {
     public init() {
     }
 
+    func lock() {
+        _lock.lock()
+    }
+
+    func unlock() {
+        _lock.unlock()
+    }
+
     /// Execute the given block while holding the lock.
     public func withLock<T> (_ body: () throws -> T) rethrows -> T {
-        _lock.lock()
-        defer { _lock.unlock() }
+        lock()
+        defer { unlock() }
         return try body()
     }
 }
@@ -35,6 +43,12 @@ enum ProcessLockError: Swift.Error {
 /// It can be used for things like serializing concurrent mutations on a shared resource
 /// by mutiple instances of a process. The `FileLock` is not thread-safe.
 public final class FileLock {
+
+    public enum LockType {
+        case exclusive
+        case shared
+    }
+
     /// File descriptor to the lock file.
   #if os(Windows)
     private var handle: HANDLE?
@@ -56,14 +70,14 @@ public final class FileLock {
     /// Try to aquire a lock. This method will block until lock the already aquired by other process.
     ///
     /// Note: This method can throw if underlying POSIX methods fail.
-    public func lock() throws {
+    public func lock(type: LockType = .exclusive) throws {
       #if os(Windows)
         if handle == nil {
-            let h = lockFile.pathString.withCString(encodedAs: UTF16.self, {
+            let h: HANDLE = lockFile.pathString.withCString(encodedAs: UTF16.self, {
                 CreateFileW(
                     $0,
                     UInt32(GENERIC_READ) | UInt32(GENERIC_WRITE),
-                    0,
+                    UInt32(FILE_SHARE_READ) | UInt32(FILE_SHARE_WRITE),
                     nil,
                     DWORD(OPEN_ALWAYS),
                     DWORD(FILE_ATTRIBUTE_NORMAL),
@@ -79,9 +93,17 @@ public final class FileLock {
         overlapped.Offset = 0
         overlapped.OffsetHigh = 0
         overlapped.hEvent = nil
-        if !LockFileEx(handle, DWORD(LOCKFILE_EXCLUSIVE_LOCK), 0,
-                       DWORD(INT_MAX), DWORD(INT_MAX), &overlapped) {
-            throw ProcessLockError.unableToAquireLock(errno: Int32(GetLastError()))
+        switch type {
+        case .exclusive:
+            if !LockFileEx(handle, DWORD(LOCKFILE_EXCLUSIVE_LOCK), 0,
+                           DWORD(INT_MAX), DWORD(INT_MAX), &overlapped) {
+                throw ProcessLockError.unableToAquireLock(errno: Int32(GetLastError()))
+            }
+        case .shared:
+            if !LockFileEx(handle, 0, 0,
+                           DWORD(INT_MAX), DWORD(INT_MAX), &overlapped) {
+                throw ProcessLockError.unableToAquireLock(errno: Int32(GetLastError()))
+            }
         }
       #else
         // Open the lock file.
@@ -94,7 +116,9 @@ public final class FileLock {
         }
         // Aquire lock on the file.
         while true {
-            if flock(fileDescriptor!, LOCK_EX) == 0 {
+            if type == .exclusive && flock(fileDescriptor!, LOCK_EX) == 0 {
+                break
+            } else if type == .shared && flock(fileDescriptor!, LOCK_SH) == 0 {
                 break
             }
             // Retry if interrupted.
@@ -129,8 +153,8 @@ public final class FileLock {
     }
 
     /// Execute the given block while holding the lock.
-    public func withLock<T>(_ body: () throws -> T) throws -> T {
-        try lock()
+    public func withLock<T>(type: LockType = .exclusive, _ body: () throws -> T) throws -> T {
+        try lock(type: type)
         defer { unlock() }
         return try body()
     }

Sources/TSCBasic/Thread.swift

@@ -64,6 +64,15 @@ final public class Thread {
             }
         }
     }
+
+    /// Causes the calling thread to yield execution to another thread.
+    public static func yield() {
+        #if os(Windows)
+          SwitchToThread()
+        #else
+          sched_yield()
+        #endif
+    }
 }
 
 #if canImport(Darwin)

Tests/TSCBasicTests/FileSystemTests.swift

@@ -602,6 +602,183 @@ class FileSystemTests: XCTestCase {
         }
       #endif
     }
+
+    func testInMemoryFileSystemFileLock() throws {
+        let fs = InMemoryFileSystem()
+        let path = AbsolutePath("/")
+        try fs.createDirectory(path)
+
+        let fileA = path.appending(component: "fileA")
+        let fileB = path.appending(component: "fileB")
+        let lockFile = path.appending(component: "lockfile")
+
+        let writerThreads = (0..<100).map { _ in
+            return Thread {
+                try! fs.withLock(on: lockFile, type: .exclusive) {
+                    // Get thr current contents of the file if any.
+                    let valueA: Int
+                    if fs.exists(fileA) {
+                        valueA = Int(try fs.readFileContents(fileA).description) ?? 0
+                    } else {
+                        valueA = 0
+                    }
+                    // Sum and write back to file.
+                    try fs.writeFileContents(fileA, bytes: ByteString(encodingAsUTF8: String(valueA + 1)))
+
+                    Thread.yield()
+
+                    // Get thr current contents of the file if any.
+                    let valueB: Int
+                    if fs.exists(fileB) {
+                        valueB = Int(try fs.readFileContents(fileB).description) ?? 0
+                    } else {
+                        valueB = 0
+                    }
+                    // Sum and write back to file.
+                    try fs.writeFileContents(fileB, bytes: ByteString(encodingAsUTF8: String(valueB + 1)))
+                }
+            }
+        }
+
+        let readerThreads = (0..<20).map { _ in
+            return Thread {
+                try! fs.withLock(on: lockFile, type: .shared) {
+                    try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
+
+                    Thread.yield()
+
+                    try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
+                }
+            }
+        }
+
+        writerThreads.forEach { $0.start() }
+        readerThreads.forEach { $0.start() }
+        writerThreads.forEach { $0.join() }
+        readerThreads.forEach { $0.join() }
+
+        try XCTAssertEqual(fs.readFileContents(fileA), "100")
+        try XCTAssertEqual(fs.readFileContents(fileB), "100")
+    }
+
+    func testLocalFileSystemFileLock() throws {
+        try withTemporaryDirectory { tempDir in
+            let fileA = tempDir.appending(component: "fileA")
+            let fileB = tempDir.appending(component: "fileB")
+            let lockFile = tempDir.appending(component: "lockfile")
+
+            let writerThreads = (0..<100).map { _ in
+                return Thread {
+                    try! localFileSystem.withLock(on: lockFile, type: .exclusive) {
+                        // Get thr current contents of the file if any.
+                        let valueA: Int
+                        if localFileSystem.exists(fileA) {
+                            valueA = Int(try localFileSystem.readFileContents(fileA).description) ?? 0
+                        } else {
+                            valueA = 0
+                        }
+                        // Sum and write back to file.
+                        try localFileSystem.writeFileContents(fileA, bytes: ByteString(encodingAsUTF8: String(valueA + 1)))
+
+                        Thread.yield()
+
+                        // Get thr current contents of the file if any.
+                        let valueB: Int
+                        if localFileSystem.exists(fileB) {
+                            valueB = Int(try localFileSystem.readFileContents(fileB).description) ?? 0
+                        } else {
+                            valueB = 0
+                        }
+                        // Sum and write back to file.
+                        try localFileSystem.writeFileContents(fileB, bytes: ByteString(encodingAsUTF8: String(valueB + 1)))
+                    }
+                }
+            }
+
+            let readerThreads = (0..<20).map { _ in
+                return Thread {
+                    try! localFileSystem.withLock(on: lockFile, type: .shared) {
+                        try XCTAssertEqual(localFileSystem.readFileContents(fileA), localFileSystem.readFileContents(fileB))
+
+                        Thread.yield()
+
+                        try XCTAssertEqual(localFileSystem.readFileContents(fileA), localFileSystem.readFileContents(fileB))
+                    }
+                }
+            }
+
+            writerThreads.forEach { $0.start() }
+            readerThreads.forEach { $0.start() }
+            writerThreads.forEach { $0.join() }
+            readerThreads.forEach { $0.join() }
+
+            try XCTAssertEqual(localFileSystem.readFileContents(fileA), "100")
+            try XCTAssertEqual(localFileSystem.readFileContents(fileB), "100")
+        }
+    }
+
+    func testRerootedFileSystemViewFileLock() throws {
+        let inMemoryFS = InMemoryFileSystem()
+        let rootPath = AbsolutePath("/tmp")
+        try inMemoryFS.createDirectory(rootPath)
+
+        let fs = RerootedFileSystemView(inMemoryFS, rootedAt: rootPath)
+        let path = AbsolutePath("/")
+        try fs.createDirectory(path)
+
+        let fileA = path.appending(component: "fileA")
+        let fileB = path.appending(component: "fileB")
+        let lockFile = path.appending(component: "lockfile")
+
+        let writerThreads = (0..<100).map { _ in
+            return Thread {
+                try! fs.withLock(on: lockFile, type: .exclusive) {
+                    // Get thr current contents of the file if any.
+                    let valueA: Int
+                    if fs.exists(fileA) {
+                        valueA = Int(try! fs.readFileContents(fileA).description) ?? 0
+                    } else {
+                        valueA = 0
+                    }
+                    // Sum and write back to file.
+                    try! fs.writeFileContents(fileA, bytes: ByteString(encodingAsUTF8: String(valueA + 1)))
+
+                    Thread.yield()
+
+                    // Get thr current contents of the file if any.
+                    let valueB: Int
+                    if fs.exists(fileB) {
+                        valueB = Int(try fs.readFileContents(fileB).description) ?? 0
+                    } else {
+                        valueB = 0
+                    }
+                    // Sum and write back to file.
+                    try fs.writeFileContents(fileB, bytes: ByteString(encodingAsUTF8: String(valueB + 1)))
+                }
+            }
+        }
+
+        let readerThreads = (0..<20).map { _ in
+            return Thread {
+                try! fs.withLock(on: lockFile, type: .shared) {
+                    try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
+
+                    Thread.yield()
+
+                    try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
+                }
+            }
+        }
+
+        writerThreads.forEach { $0.start() }
+        readerThreads.forEach { $0.start() }
+        writerThreads.forEach { $0.join() }
+        readerThreads.forEach { $0.join() }
+
+        try XCTAssertEqual(fs.readFileContents(fileA), "100")
+        try XCTAssertEqual(fs.readFileContents(fileB), "100")
+    }
+
 }
 
 /// Helper method to test file tree removal method on the given file system.