Skip to content

Commit a77e5a0

Browse files
neonichuneonichu
authored
Merge pull request swiftlang#139 from tgymnich/move-lock-to-file-system
Lock Improvements for [SR-12851]
2 parents 859b5d5 + 891cef9 commit a77e5a0

File tree

6 files changed

+326
-11
lines changed

6 files changed

+326
-11
lines changed

‎Sources/TSCBasic/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ add_library(TSCBasic
4949
Thread.swift
5050
Tuple.swift
5151
misc.swift)
52+
5253
target_compile_options(TSCBasic PUBLIC
5354
# Don't use GNU strerror_r on Android.
5455
"$<$<PLATFORM_ID:Android>:SHELL:-Xcc -U_GNU_SOURCE>"

‎Sources/TSCBasic/FileSystem.swift

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import TSCLibc
1212
import Foundation
13+
import Dispatch
1314

1415
public enum FileSystemError: Swift.Error {
1516
/// Access to the path is denied.
@@ -200,6 +201,9 @@ public protocol FileSystem: class {
200201

201202
/// Move a file or directory.
202203
func move(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws
204+
205+
/// Execute the given block while holding the lock.
206+
func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T
203207
}
204208

205209
/// Convenience implementations (default arguments aren't permitted in protocol
@@ -240,6 +244,10 @@ public extension FileSystem {
240244
func getFileInfo(_ path: AbsolutePath) throws -> FileInfo {
241245
throw FileSystemError.unsupported
242246
}
247+
248+
func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T {
249+
throw FileSystemError.unsupported
250+
}
243251
}
244252

245253
/// Concrete FileSystem implementation which communicates with the local file system.
@@ -450,6 +458,11 @@ private class LocalFileSystem: FileSystem {
450458
guard !exists(destinationPath) else { throw FileSystemError.alreadyExistsAtDestination }
451459
try FileManager.default.moveItem(at: sourcePath.asURL, to: destinationPath.asURL)
452460
}
461+
462+
func withLock<T>(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T {
463+
let lock = FileLock(name: path.basename, cachePath: path.parentDirectory)
464+
return try lock.withLock(type: type, body)
465+
}
453466
}
454467

455468
// FIXME: This class does not yet support concurrent mutation safely.
@@ -499,9 +512,21 @@ public class InMemoryFileSystem: FileSystem {
499512
return contents
500513
}
501514
}
515+
// Used to ensure that DispatchQueues are releassed when they are no longer in use.
516+
private struct WeakReference<Value: AnyObject> {
517+
weak var reference: Value?
518+
519+
init(_ value: Value?) {
520+
self.reference = value
521+
}
522+
}
502523

503524
/// The root filesytem.
504525
private var root: Node
526+
/// A map that keeps weak references to all locked files.
527+
private var lockFiles = Dictionary<AbsolutePath, WeakReference<DispatchQueue>>()
528+
/// Used to access lockFiles in a thread safe manner.
529+
private let lockFilesLock = Lock()
505530

506531
public init() {
507532
root = Node(.directory(DirectoryContents()))
@@ -760,6 +785,21 @@ public class InMemoryFileSystem: FileSystem {
760785

761786
contents.entries[sourcePath.basename] = nil
762787
}
788+
789+
public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T {
790+
791+
let fileQueue: DispatchQueue = lockFilesLock.withLock {
792+
if let queueReference = lockFiles[path], let queue = queueReference.reference {
793+
return queue
794+
} else {
795+
let queue = DispatchQueue(label: "org.swift.swiftpm.in-memory-file-system.file-queue", attributes: .concurrent)
796+
lockFiles[path] = WeakReference(queue)
797+
return queue
798+
}
799+
}
800+
801+
return try fileQueue.sync(flags: type == .exclusive ? .barrier : .init() , execute: body)
802+
}
763803
}
764804

765805
/// A rerooted view on an existing FileSystem.
@@ -864,6 +904,10 @@ public class RerootedFileSystemView: FileSystem {
864904
public func move(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws {
865905
try underlyingFileSystem.move(from: formUnderlyingPath(sourcePath), to: formUnderlyingPath(sourcePath))
866906
}
907+
908+
public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType = .exclusive, _ body: () throws -> T) throws -> T {
909+
return try underlyingFileSystem.withLock(on: formUnderlyingPath(path), type: type, body)
910+
}
867911
}
868912

869913
/// Public access to the local FS proxy.

‎Sources/TSCBasic/Lock.swift

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,18 @@ public struct Lock {
1919
public init() {
2020
}
2121

22+
func lock() {
23+
_lock.lock()
24+
}
25+
26+
func unlock() {
27+
_lock.unlock()
28+
}
29+
2230
/// Execute the given block while holding the lock.
2331
public func withLock<T> (_ body: () throws -> T) rethrows -> T {
24-
_lock.lock()
25-
defer { _lock.unlock() }
32+
lock()
33+
defer { unlock() }
2634
return try body()
2735
}
2836
}
@@ -35,6 +43,12 @@ enum ProcessLockError: Swift.Error {
3543
/// It can be used for things like serializing concurrent mutations on a shared resource
3644
/// by mutiple instances of a process. The `FileLock` is not thread-safe.
3745
public final class FileLock {
46+
47+
public enum LockType {
48+
case exclusive
49+
case shared
50+
}
51+
3852
/// File descriptor to the lock file.
3953
#if os(Windows)
4054
private var handle: HANDLE?
@@ -56,14 +70,14 @@ public final class FileLock {
5670
/// Try to aquire a lock. This method will block until lock the already aquired by other process.
5771
///
5872
/// Note: This method can throw if underlying POSIX methods fail.
59-
public func lock() throws {
73+
public func lock(type: LockType = .exclusive) throws {
6074
#if os(Windows)
6175
if handle == nil {
62-
let h = lockFile.pathString.withCString(encodedAs: UTF16.self, {
76+
let h: HANDLE = lockFile.pathString.withCString(encodedAs: UTF16.self, {
6377
CreateFileW(
6478
$0,
6579
UInt32(GENERIC_READ) | UInt32(GENERIC_WRITE),
66-
0,
80+
UInt32(FILE_SHARE_READ) | UInt32(FILE_SHARE_WRITE),
6781
nil,
6882
DWORD(OPEN_ALWAYS),
6983
DWORD(FILE_ATTRIBUTE_NORMAL),
@@ -79,9 +93,17 @@ public final class FileLock {
7993
overlapped.Offset = 0
8094
overlapped.OffsetHigh = 0
8195
overlapped.hEvent = nil
82-
if !LockFileEx(handle, DWORD(LOCKFILE_EXCLUSIVE_LOCK), 0,
83-
DWORD(INT_MAX), DWORD(INT_MAX), &overlapped) {
84-
throw ProcessLockError.unableToAquireLock(errno: Int32(GetLastError()))
96+
switch type {
97+
case .exclusive:
98+
if !LockFileEx(handle, DWORD(LOCKFILE_EXCLUSIVE_LOCK), 0,
99+
DWORD(INT_MAX), DWORD(INT_MAX), &overlapped) {
100+
throw ProcessLockError.unableToAquireLock(errno: Int32(GetLastError()))
101+
}
102+
case .shared:
103+
if !LockFileEx(handle, 0, 0,
104+
DWORD(INT_MAX), DWORD(INT_MAX), &overlapped) {
105+
throw ProcessLockError.unableToAquireLock(errno: Int32(GetLastError()))
106+
}
85107
}
86108
#else
87109
// Open the lock file.
@@ -94,7 +116,9 @@ public final class FileLock {
94116
}
95117
// Aquire lock on the file.
96118
while true {
97-
if flock(fileDescriptor!, LOCK_EX) == 0 {
119+
if type == .exclusive && flock(fileDescriptor!, LOCK_EX) == 0 {
120+
break
121+
} else if type == .shared && flock(fileDescriptor!, LOCK_SH) == 0 {
98122
break
99123
}
100124
// Retry if interrupted.
@@ -129,8 +153,8 @@ public final class FileLock {
129153
}
130154

131155
/// Execute the given block while holding the lock.
132-
public func withLock<T>(_ body: () throws -> T) throws -> T {
133-
try lock()
156+
public func withLock<T>(type: LockType = .exclusive, _ body: () throws -> T) throws -> T {
157+
try lock(type: type)
134158
defer { unlock() }
135159
return try body()
136160
}

‎Sources/TSCBasic/Thread.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ final public class Thread {
6464
}
6565
}
6666
}
67+
68+
/// Causes the calling thread to yield execution to another thread.
69+
public static func yield() {
70+
#if os(Windows)
71+
SwitchToThread()
72+
#else
73+
sched_yield()
74+
#endif
75+
}
6776
}
6877

6978
#if canImport(Darwin)

‎Tests/TSCBasicTests/FileSystemTests.swift

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,183 @@ class FileSystemTests: XCTestCase {
602602
}
603603
#endif
604604
}
605+
606+
func testInMemoryFileSystemFileLock() throws {
607+
let fs = InMemoryFileSystem()
608+
let path = AbsolutePath("/")
609+
try fs.createDirectory(path)
610+
611+
let fileA = path.appending(component: "fileA")
612+
let fileB = path.appending(component: "fileB")
613+
let lockFile = path.appending(component: "lockfile")
614+
615+
let writerThreads = (0..<100).map { _ in
616+
return Thread {
617+
try! fs.withLock(on: lockFile, type: .exclusive) {
618+
// Get thr current contents of the file if any.
619+
let valueA: Int
620+
if fs.exists(fileA) {
621+
valueA = Int(try fs.readFileContents(fileA).description) ?? 0
622+
} else {
623+
valueA = 0
624+
}
625+
// Sum and write back to file.
626+
try fs.writeFileContents(fileA, bytes: ByteString(encodingAsUTF8: String(valueA + 1)))
627+
628+
Thread.yield()
629+
630+
// Get thr current contents of the file if any.
631+
let valueB: Int
632+
if fs.exists(fileB) {
633+
valueB = Int(try fs.readFileContents(fileB).description) ?? 0
634+
} else {
635+
valueB = 0
636+
}
637+
// Sum and write back to file.
638+
try fs.writeFileContents(fileB, bytes: ByteString(encodingAsUTF8: String(valueB + 1)))
639+
}
640+
}
641+
}
642+
643+
let readerThreads = (0..<20).map { _ in
644+
return Thread {
645+
try! fs.withLock(on: lockFile, type: .shared) {
646+
try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
647+
648+
Thread.yield()
649+
650+
try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
651+
}
652+
}
653+
}
654+
655+
writerThreads.forEach { $0.start() }
656+
readerThreads.forEach { $0.start() }
657+
writerThreads.forEach { $0.join() }
658+
readerThreads.forEach { $0.join() }
659+
660+
try XCTAssertEqual(fs.readFileContents(fileA), "100")
661+
try XCTAssertEqual(fs.readFileContents(fileB), "100")
662+
}
663+
664+
func testLocalFileSystemFileLock() throws {
665+
try withTemporaryDirectory { tempDir in
666+
let fileA = tempDir.appending(component: "fileA")
667+
let fileB = tempDir.appending(component: "fileB")
668+
let lockFile = tempDir.appending(component: "lockfile")
669+
670+
let writerThreads = (0..<100).map { _ in
671+
return Thread {
672+
try! localFileSystem.withLock(on: lockFile, type: .exclusive) {
673+
// Get thr current contents of the file if any.
674+
let valueA: Int
675+
if localFileSystem.exists(fileA) {
676+
valueA = Int(try localFileSystem.readFileContents(fileA).description) ?? 0
677+
} else {
678+
valueA = 0
679+
}
680+
// Sum and write back to file.
681+
try localFileSystem.writeFileContents(fileA, bytes: ByteString(encodingAsUTF8: String(valueA + 1)))
682+
683+
Thread.yield()
684+
685+
// Get thr current contents of the file if any.
686+
let valueB: Int
687+
if localFileSystem.exists(fileB) {
688+
valueB = Int(try localFileSystem.readFileContents(fileB).description) ?? 0
689+
} else {
690+
valueB = 0
691+
}
692+
// Sum and write back to file.
693+
try localFileSystem.writeFileContents(fileB, bytes: ByteString(encodingAsUTF8: String(valueB + 1)))
694+
}
695+
}
696+
}
697+
698+
let readerThreads = (0..<20).map { _ in
699+
return Thread {
700+
try! localFileSystem.withLock(on: lockFile, type: .shared) {
701+
try XCTAssertEqual(localFileSystem.readFileContents(fileA), localFileSystem.readFileContents(fileB))
702+
703+
Thread.yield()
704+
705+
try XCTAssertEqual(localFileSystem.readFileContents(fileA), localFileSystem.readFileContents(fileB))
706+
}
707+
}
708+
}
709+
710+
writerThreads.forEach { $0.start() }
711+
readerThreads.forEach { $0.start() }
712+
writerThreads.forEach { $0.join() }
713+
readerThreads.forEach { $0.join() }
714+
715+
try XCTAssertEqual(localFileSystem.readFileContents(fileA), "100")
716+
try XCTAssertEqual(localFileSystem.readFileContents(fileB), "100")
717+
}
718+
}
719+
720+
func testRerootedFileSystemViewFileLock() throws {
721+
let inMemoryFS = InMemoryFileSystem()
722+
let rootPath = AbsolutePath("/tmp")
723+
try inMemoryFS.createDirectory(rootPath)
724+
725+
let fs = RerootedFileSystemView(inMemoryFS, rootedAt: rootPath)
726+
let path = AbsolutePath("/")
727+
try fs.createDirectory(path)
728+
729+
let fileA = path.appending(component: "fileA")
730+
let fileB = path.appending(component: "fileB")
731+
let lockFile = path.appending(component: "lockfile")
732+
733+
let writerThreads = (0..<100).map { _ in
734+
return Thread {
735+
try! fs.withLock(on: lockFile, type: .exclusive) {
736+
// Get thr current contents of the file if any.
737+
let valueA: Int
738+
if fs.exists(fileA) {
739+
valueA = Int(try! fs.readFileContents(fileA).description) ?? 0
740+
} else {
741+
valueA = 0
742+
}
743+
// Sum and write back to file.
744+
try! fs.writeFileContents(fileA, bytes: ByteString(encodingAsUTF8: String(valueA + 1)))
745+
746+
Thread.yield()
747+
748+
// Get thr current contents of the file if any.
749+
let valueB: Int
750+
if fs.exists(fileB) {
751+
valueB = Int(try fs.readFileContents(fileB).description) ?? 0
752+
} else {
753+
valueB = 0
754+
}
755+
// Sum and write back to file.
756+
try fs.writeFileContents(fileB, bytes: ByteString(encodingAsUTF8: String(valueB + 1)))
757+
}
758+
}
759+
}
760+
761+
let readerThreads = (0..<20).map { _ in
762+
return Thread {
763+
try! fs.withLock(on: lockFile, type: .shared) {
764+
try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
765+
766+
Thread.yield()
767+
768+
try XCTAssertEqual(fs.readFileContents(fileA), fs.readFileContents(fileB))
769+
}
770+
}
771+
}
772+
773+
writerThreads.forEach { $0.start() }
774+
readerThreads.forEach { $0.start() }
775+
writerThreads.forEach { $0.join() }
776+
readerThreads.forEach { $0.join() }
777+
778+
try XCTAssertEqual(fs.readFileContents(fileA), "100")
779+
try XCTAssertEqual(fs.readFileContents(fileB), "100")
780+
}
781+
605782
}
606783

607784
/// Helper method to test file tree removal method on the given file system.

0 commit comments

Comments
 (0)