Fix cross-terminal hyperlink GC and make TinyAtom thread-safe (#611)

Author: klc

Sources/SwiftTerm/CharData.swift

@@ -190,9 +190,9 @@ public struct Attribute: Equatable, Hashable {
 /// it could in theory be changed to be 24 bits without much trouble
 public struct TinyAtom {
     var code: UInt16
-    static var map: [UInt16:Any] = [:]
-    static var lastUsed: Int = 0
-    static var lastCollected: Int = 0
+    private static let lock = NSLock()
+    private static var map: [UInt16:Any] = [:]
+    private static var lastUsed: UInt16 = 0
     static let empty = TinyAtom (code: 0)
 
     private init(code: UInt16)
@@ -202,17 +202,30 @@ public struct TinyAtom {
 
     /// Returns the TinyAtom associated with the specified url, or nil if we ran out of space
     public static func lookup (value: Any) -> TinyAtom? {
-        let next = lastUsed + 1
-        if next < UInt16.max {
-            map [UInt16 (next)] = value
-            lastUsed = next
-            return TinyAtom (code: UInt16 (next))
+        lock.lock()
+        defer { lock.unlock() }
+
+        guard lastUsed < UInt16.max - 1 else {
+            return nil
         }
-        return nil
+        lastUsed += 1
+        let code = lastUsed
+
+        map [code] = value
+        return TinyAtom (code: code)
     }
 
     public static func release(code: UInt16) {
-        map.removeValue(forKey: code)
+        release(codes: [code])
+    }
+
+    static func release<S: Sequence>(codes: S) where S.Element == UInt16 {
+        lock.lock()
+        defer { lock.unlock() }
+
+        for code in codes where code != 0 {
+            map.removeValue(forKey: code)
+        }
     }
 
     /// Returns the target for the TinyAtom
@@ -221,6 +234,8 @@ public struct TinyAtom {
             if code == 0 {
                 return nil
             }
+            TinyAtom.lock.lock()
+            defer { TinyAtom.lock.unlock() }
             return TinyAtom.map [code]
         }
     }

Sources/SwiftTerm/Terminal.swift

@@ -706,6 +706,10 @@ open class Terminal {
         setup()
     }
 
+    deinit {
+        TinyAtom.release(codes: payloadCodes)
+    }
+
     /// Installs the new colors as the default colors and recomputes the
     /// current and ansi palette.   This will not change the UI layer, for that it is better
     /// to call the `installColors` method on `TerminalView`, which will
@@ -1759,7 +1763,8 @@ open class Terminal {
     }
 
     var hyperLinkTracking: (start: Position, payload: String)? = nil
-    
+    private var payloadCodes = Set<UInt16>()
+
     func oscHyperlink (_ data: ArraySlice<UInt8>)
     {
         let buffer = self.buffer
@@ -1768,6 +1773,7 @@ open class Terminal {
             if let hlt = hyperLinkTracking {
                 let str = hlt.payload
                 if let urlToken = TinyAtom.lookup (value: str) {
+                    payloadCodes.insert(urlToken.code)
                     //print ("Setting the text from \(hlt.start) to \(buffer.x) on line \(buffer.y+buffer.yBase) to \(str)")
 
                     // Between the time the flag was set, and now `y` might have changed negatively,
@@ -5120,8 +5126,7 @@ open class Terminal {
      * available by scrolling.
      */
     public func garbageCollectPayload() {
-        // stop right away if there is nothing to collect
-        if TinyAtom.lastCollected == TinyAtom.lastUsed {
+        if payloadCodes.isEmpty {
             return
         }
 
@@ -5141,16 +5146,10 @@ open class Terminal {
             }
         }
 
-        // since we create atoms in order we expect them to run out of use
-        // in order as well and stop with first atom that is still in use
-        for code in UInt16(TinyAtom.lastCollected + 1)...UInt16(TinyAtom.lastUsed) {
-            if used.contains(code) {
-                // code still in use
-                break
-            }
-            
-            TinyAtom.lastCollected = Int(code)
-            TinyAtom.release(code: code)
+        let released = payloadCodes.subtracting(used)
+        if !released.isEmpty {
+            TinyAtom.release(codes: released)
+            payloadCodes.subtract(released)
         }
     }
 

Tests/SwiftTermTests/LinkLookupTests.swift

@@ -42,6 +42,48 @@ final class LinkLookupTests: TerminalDelegate {
         #expect(link == "https://example.com")
     }
 
+    @Test func testGarbageCollectionDoesNotReleaseAnotherTerminalsPayload() {
+        let first = Terminal(delegate: self, options: TerminalOptions(cols: 20, rows: 1))
+        let second = Terminal(delegate: self, options: TerminalOptions(cols: 20, rows: 1))
+
+        first.feed(text: "\u{1b}]8;;https://first.example\u{07}first\u{1b}]8;;\u{07}")
+        second.feed(text: "\u{1b}]8;;https://second.example\u{07}second\u{1b}]8;;\u{07}")
+
+        let firstAtom = first.displayBuffer.lines[0][0].payload
+        let secondAtom = second.displayBuffer.lines[0][0].payload
+        #expect(firstAtom.target != nil)
+        #expect(secondAtom.target != nil)
+
+        first.feed(text: "\u{1b}[2J")
+        first.garbageCollectPayload()
+
+        #expect(firstAtom.target == nil)
+        #expect(secondAtom.target != nil)
+    }
+
+    @Test func testTinyAtomConcurrentLookupAndRelease() async {
+        let allValuesMatched = await withTaskGroup(of: Bool.self, returning: Bool.self) { group in
+            for value in 0..<1_000 {
+                group.addTask {
+                    guard let atom = TinyAtom.lookup(value: value) else {
+                        return false
+                    }
+                    let matched = atom.target as? Int == value
+                    TinyAtom.release(code: atom.code)
+                    return matched
+                }
+            }
+
+            var result = true
+            for await matched in group {
+                result = result && matched
+            }
+            return result
+        }
+
+        #expect(allValuesMatched)
+    }
+
     @Test func testImplicitUrlLookup() {
         let terminal = Terminal(delegate: self, options: TerminalOptions(cols: 40, rows: 1))
         terminal.feed(text: "https://example.com tail")