Reduced critical section under lock

roulpriya · roulpriya · commit 1d67cb7dfa90 · 2025-05-29T11:17:25.000+05:30
diff --git a/Sources/Swiftly/Install.swift b/Sources/Swiftly/Install.swift
@@ -92,32 +92,6 @@ struct Install: SwiftlyCommand {
         }
         try await validateLinked(ctx)
 
-        let lockFile = self.swiftlyHomeDir(ctx) / "swiftly.lock"
-        let fileLock: SwiftlyCore.FileLock
-
-        do {
-            fileLock = try SwiftlyCore.FileLock(at: lockFile.string)
-        } catch {
-            print("ERROR: Failed to create lock file: \(error)")
-            Foundation.exit(42)
-        }
-
-        defer {
-            do {
-                try fileLock.unlock()
-            } catch {
-                print("WARNING: Failed to unlock file: \(error)")
-            }
-        }
-
-        if self.root.verbose {
-            await ctx.print("Attempting to acquire installation lock...")
-        }
-        guard fileLock.waitForLock(timeout: 300, pollingInterval: 2) else {
-            print("ERROR: Failed to acquire lock on file: \(lockFile.string) after 300 seconds")
-            Foundation.exit(42)
-        }
-
         var config = try await Config.load(ctx)
         let toolchainVersion = try await Self.determineToolchainVersion(ctx, version: self.version, config: &config)
 
@@ -376,33 +350,51 @@ struct Install: SwiftlyCommand {
                 )
             }
 
-            try await Swiftly.currentPlatform.install(ctx, from: tmpFile, version: version, verbose: verbose)
+            let lockFile = Swiftly.currentPlatform.swiftlyHomeDir(ctx) / "swiftly.lock"
+            if verbose {
+                await ctx.print("Attempting to acquire installation lock at \(lockFile) ...")
+            }
+
+            let (pathChanged, newConfig) = try await withLock(lockFile) {
+                if verbose {
+                    await ctx.print("Acquired installation lock.")
+                }
 
-            let pathChanged = try await Self.setupProxies(
-                ctx,
-                version: version,
-                verbose: verbose,
-                assumeYes: assumeYes
-            )
+                var config = try await Config.load(ctx)
 
-            config.installedToolchains.insert(version)
+                try await Swiftly.currentPlatform.install(
+                    ctx, from: tmpFile,
+                    version: version,
+                    verbose: verbose
+                )
 
-            try config.save(ctx)
+                let pathChanged = try await Self.setupProxies(
+                    ctx,
+                    version: version,
+                    verbose: verbose,
+                    assumeYes: assumeYes
+                )
 
-            // If this is the first installed toolchain, mark it as in-use regardless of whether the
-            // --use argument was provided.
-            if useInstalledToolchain {
-                try await Use.execute(ctx, version, globalDefault: false, &config)
-            }
+                config.installedToolchains.insert(version)
 
-            // We always update the global default toolchain if there is none set. This could
-            //  be the only toolchain that is installed, which makes it the only choice.
-            if config.inUse == nil {
-                config.inUse = version
                 try config.save(ctx)
-                await ctx.print("The global default toolchain has been set to `\(version)`")
-            }
 
+                // If this is the first installed toolchain, mark it as in-use regardless of whether the
+                // --use argument was provided.
+                if useInstalledToolchain {
+                    try await Use.execute(ctx, version, globalDefault: false, &config)
+                }
+
+                // We always update the global default toolchain if there is none set. This could
+                //  be the only toolchain that is installed, which makes it the only choice.
+                if config.inUse == nil {
+                    config.inUse = version
+                    try config.save(ctx)
+                    await ctx.print("The global default toolchain has been set to `\(version)`")
+                }
+                return (pathChanged, config)
+            }
+            config = newConfig
             await ctx.print("\(version) installed successfully!")
             return (postInstallScript, pathChanged)
         }
diff --git a/Sources/SwiftlyCore/FileLock.swift b/Sources/SwiftlyCore/FileLock.swift
@@ -1,99 +1,83 @@
 import Foundation
+import SystemPackage
 
 /**
- * A non-blocking file lock implementation with polling capability.
+ * A non-blocking file lock implementation using file creation as locking mechanism.
  * Use case: When installing multiple Swiftly instances on the same machine,
  * one should acquire the lock while others poll until it becomes available.
  */
 
-#if os(macOS)
-import Darwin.C
-#elseif os(Linux)
-import Glibc
-#endif
-
-public struct FileLock {
-    let filePath: String
-
-    let fileHandle: FileHandle
-
-    public static let defaultPollingInterval: TimeInterval = 1.0
+public actor FileLock {
+    let filePath: FilePath
+    private var isLocked = false
 
+    public static let defaultPollingInterval: TimeInterval = 1
     public static let defaultTimeout: TimeInterval = 300.0
 
-    public init(at filePath: String) throws {
-        self.filePath = filePath
-
-        if !FileManager.default.fileExists(atPath: filePath) {
-            FileManager.default.createFile(atPath: filePath, contents: nil)
-        }
-
-        self.fileHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: filePath))
+    public init(at path: FilePath) {
+        self.filePath = path
     }
 
-    public func tryLock() -> Bool {
-        self.fileHandle.tryLockFile()
+    public func tryLock() async -> Bool {
+        do {
+            guard !self.isLocked else { return true }
+
+            guard !(try await FileSystem.exists(atPath: self.filePath)) else {
+                return false
+            }
+            // Create the lock file with exclusive permissions
+            try await FileSystem.create(.mode(0o600), file: self.filePath, contents: nil)
+            self.isLocked = true
+            return true
+        } catch {
+            return false
+        }
     }
 
     public func waitForLock(
         timeout: TimeInterval = FileLock.defaultTimeout,
         pollingInterval: TimeInterval = FileLock.defaultPollingInterval
-    ) -> Bool {
-        let startTime = Date()
+    ) async -> Bool {
+        let start = Date()
 
-        if self.tryLock() {
-            return true
-        }
-
-        while Date().timeIntervalSince(startTime) < timeout {
-            Thread.sleep(forTimeInterval: pollingInterval)
-
-            if self.tryLock() {
+        while Date().timeIntervalSince(start) < timeout {
+            if await self.tryLock() {
                 return true
             }
+            try? await Task.sleep(for: .seconds(pollingInterval))
         }
 
         return false
     }
 
-    public func unlock() throws {
-        guard self.fileHandle != nil else { return }
-        try self.fileHandle.unlockFile()
-        try self.fileHandle.close()
+    public func unlock() async throws {
+        guard self.isLocked else { return }
+
+        try await FileSystem.remove(atPath: self.filePath)
+        self.isLocked = false
     }
 }
 
-extension FileHandle {
-    func tryLockFile() -> Bool {
-        let fd = self.fileDescriptor
-        var flock = flock()
-        flock.l_type = Int16(F_WRLCK)
-        flock.l_whence = Int16(SEEK_SET)
-        flock.l_start = 0
-        flock.l_len = 0
+public func withLock<T>(
+    _ lockFile: FilePath,
+    timeout: TimeInterval = FileLock.defaultTimeout,
+    pollingInterval: TimeInterval = FileLock.defaultPollingInterval,
+    action: @escaping () async throws -> T
+) async throws -> T {
+    let lock = FileLock(at: lockFile)
+    guard await lock.waitForLock(timeout: timeout, pollingInterval: pollingInterval) else {
+        throw SwiftlyError(message: "Failed to acquire file lock at \(lock.filePath)")
+    }
 
-        if fcntl(fd, F_SETLK, &flock) == -1 {
-            if errno == EACCES || errno == EAGAIN {
-                return false
-            } else {
-                fputs("Unexpected lock error: \(String(cString: strerror(errno)))\n", stderr)
-                return false
+    defer {
+        Task {
+            do {
+                try await lock.unlock()
+            } catch {
+                print("WARNING: Failed to unlock file: \(error)")
             }
         }
-        return true
     }
 
-    func unlockFile() throws {
-        let fd = self.fileDescriptor
-        var flock = flock()
-        flock.l_type = Int16(F_UNLCK)
-        flock.l_whence = Int16(SEEK_SET)
-        flock.l_start = 0
-        flock.l_len = 0
-
-        if fcntl(fd, F_SETLK, &flock) == -1 {
-            throw SwiftlyError(
-                message: "Failed to unlock file: \(String(cString: strerror(errno)))")
-        }
-    }
+    return try await action()
 }
