Add Sendable requirements on vulnerable closure types

SourceDocs/User Guide.md

@@ -597,6 +597,16 @@ RubyGateway caches intern'ed Ruby strings - you can access the cache using
 Note that when you call the Ruby API and Ruby raises an exception, the process
 immediately crashes unless you are running inside `rb_protect()` or equivalent.
 
+## Swift Concurrency
+
+Sendable annotations and checking are mostly complete.  The parts remaining are
+* `RbBlockCallback` - Swift doesn't understand @Sendable typealiases.
+* `RbMethodCallback` and related - Swift doesn't understand Sendable method
+  references.
+
+Despite the lack of `Sendable` requirement on these closure types they should
+be treated as such if you are using Ruby across multiple threads.
+
 ### Garbage collection
 
 The main risk using the `libruby` API is that GC happens too early on objects

Sources/RubyGateway/RbGateway.swift

@@ -59,7 +59,6 @@ internal import RubyGatewayHelpers
 /// and `RbObject.defineSingletonMethod(...)`.
 ///
 public final class RbGateway: RbObjectAccess, @unchecked Sendable {
-
     /// The VM - not initialized until `setup()` is called.
     static let vm = RbVM()
 
@@ -164,7 +163,8 @@ public final class RbGateway: RbObjectAccess, @unchecked Sendable {
         try name.checkRubyInstanceVarName()
 
         let ivarWorkaroundName = "$RbGatewayTopSelfIvarWorkaround"
-        try defineGlobalVar(ivarWorkaroundName, get: { newValue.rubyObject })
+        let rubyObject = newValue.rubyObject
+        try defineGlobalVar(ivarWorkaroundName, get: { rubyObject })
         return try eval(ruby: "\(name) = \(ivarWorkaroundName)")
     }
 }

Sources/RubyGateway/RbGlobalVar.swift

@@ -96,7 +96,7 @@ extension RbGateway {
     /// - throws: `RbError.badIdentifier(type:id:)` if `name` is bad; some other kind of error if Ruby is
     ///           not working.
     public func defineGlobalVar<T: RbObjectConvertible>(_ name: String,
-                                                        get: @escaping () -> T) throws {
+                                                        get: @Sendable @escaping () -> T) throws {
         try setup()
         try name.checkRubyGlobalVarName()
         RbGlobalVar.create(name: name, get: get, set: nil)
@@ -119,8 +119,8 @@ extension RbGateway {
     /// - throws: `RbError.badIdentifier(type:id:)` if `name` is bad; some other kind of error if Ruby is
     ///           not working.
     public func defineGlobalVar<T: RbObjectConvertible>(_ name: String,
-                                                        get: @escaping () -> T,
-                                                        set: @escaping (T) throws -> Void) throws {
+                                                        get: @Sendable @escaping () -> T,
+                                                        set: @Sendable @escaping (T) throws -> Void) throws {
         try setup()
         try name.checkRubyGlobalVarName()
         RbGlobalVar.create(name: name, get: get, set: set)

Sources/RubyGateway/RbMethod.swift

@@ -31,6 +31,9 @@ internal import RubyGatewayHelpers
 // dynamic dispatch order.  So we can search this property looking for a match.
 // OK - not THAT bad!
 
+// XXX these guys all ought to be Sendable, but Swift is broken wrt Sendable and
+// XXX method references....
+
 /// The function signature for a Ruby method implemented as a Swift free function
 /// or closure.
 ///
@@ -59,7 +62,7 @@ public typealias RbMethodCallback = (RbObject, RbMethod) throws -> RbObject
 /// You can throw an `RbException` to raise a Ruby exception instead of returning
 /// normally from the method.  Throwing another type gets wrapped up in an
 /// `RbException` and raised as a Ruby runtime exception.
-public typealias RbBoundMethodCallback<SwiftPeer: AnyObject, Return: RbObjectConvertible> =
+public typealias RbBoundMethodCallback<SwiftPeer: AnyObject, Return: RbObjectConvertible & Sendable> =
     (SwiftPeer) -> (RbMethod) throws -> Return
 
 /// The function signature for a Ruby method implemented as a Swift method of
@@ -189,7 +192,7 @@ private struct RbMethodDispatch {
 ///
 /// You do not create instances of this type: instead, RubyGateway creates
 /// instances and passes them to method callbacks.
-public struct RbMethod {
+public struct RbMethod: Sendable {
     /// The object against which the method has been invoked.
     public let rubySelf: RbObject
     /// The arguments passed to the method, decoded according to the method's `RbMethodArgsSpec`.
@@ -288,7 +291,7 @@ extension Array {
 /// The various types of argument passed to a Ruby method implemented in Swift.
 ///
 /// Available via `RbMethod.args` when the method is invoked.
-public struct RbMethodArgs {
+public struct RbMethodArgs: Sendable {
     /// The mandatory positional arguments to the method, comprising the
     /// leading mandatory arguments followed by the trailing mandatory arguments.
     public let mandatory: [RbObject]
@@ -317,11 +320,11 @@ public struct RbMethodArgs {
 ///
 /// If you want to say "accept any number of arguments" then write
 /// `RbMethodArgsSpec(supportsSplat: true)` and access the arguments via `method.args.splatted`.
-public struct RbMethodArgsSpec {
+public struct RbMethodArgsSpec: Sendable {
     /// The number of leading mandatory positional arguments.
     public let leadingMandatoryCount: Int
     /// Default values for all optional positional arguments.
-    public let optionalValues: [() -> RbObject]
+    public let optionalValues: [@Sendable () -> RbObject]
     /// The number of optional positional arguments.
     public var optionalCount: Int {
         optionalValues.count
@@ -337,7 +340,7 @@ public struct RbMethodArgsSpec {
     /// Names of mandatory keyword arguments.
     public let mandatoryKeywords: Set<String>
     /// Names and default values of optional keyword arguments.
-    public let optionalKeywordValues: [String : () -> RbObject]
+    public let optionalKeywordValues: [String : @Sendable () -> RbObject]
     /// Does the method support keyword arguments?
     public var supportsKeywords: Bool {
         mandatoryKeywords.count > 0 || optionalKeywordValues.count > 0
@@ -360,6 +363,11 @@ public struct RbMethodArgsSpec {
 
     /// Create a new method arguments specification.
     ///
+    /// The default values here are evaluated lazily: each time the method is invoked and requires the
+    /// default argument because caller has not provided it, the `RbObjectConvertible.rubyObject`
+    /// is evaluated and provided to the method.  In the case of constant defaults this is unobservable but
+    /// does give correct behaviour if you actually supply something that turns into a Ruby expression.
+    ///
     /// - Parameters:
     ///   - leadingMandatoryCount: The number of leading mandatory positional arguments,
     ///     none by default.
@@ -373,18 +381,23 @@ public struct RbMethodArgsSpec {
     ///   - requiresBlock: Whether the method requires a block, `false` by default.  If this is
     ///     `true` then the method may or may not be called with a block.
     public init(leadingMandatoryCount: Int = 0,
-                optionalValues: [(any RbObjectConvertible)?] = [],
+                optionalValues: [(any RbObjectConvertible & Sendable)?] = [],
                 supportsSplat: Bool = false,
                 trailingMandatoryCount: Int = 0,
                 mandatoryKeywords: Set<String> = [],
-                optionalKeywordValues: [String: (any RbObjectConvertible)?] = [:],
+                optionalKeywordValues: [String: (any RbObjectConvertible & Sendable)?] = [:],
                 requiresBlock: Bool = false) {
         self.leadingMandatoryCount = leadingMandatoryCount
-        self.optionalValues = optionalValues.map { val in { val.rubyObject } }
+
+        func lazyEvaluation(_ o: (any RbObjectConvertible & Sendable)?) -> @Sendable () -> RbObject {
+            { o.map { $0.rubyObject } ?? .nilObject }
+        }
+
+        self.optionalValues = optionalValues.map { lazyEvaluation($0) }
         self.supportsSplat = supportsSplat
         self.trailingMandatoryCount = trailingMandatoryCount
         self.mandatoryKeywords = mandatoryKeywords
-        self.optionalKeywordValues = optionalKeywordValues.mapValues { val in { val.rubyObject } }
+        self.optionalKeywordValues = optionalKeywordValues.mapValues { lazyEvaluation($0) }
         self.requiresBlock = requiresBlock
     }
 
@@ -651,7 +664,7 @@ extension RbObject {
     ///   - method: The Swift method to call to fulfill the Ruby method.
     /// - Throws: `RbError.badIdentifier(type:id:)` if `name` is bad.
     ///           `RbError.badType(...)` if the object is not a class.
-    public func defineMethod<SwiftPeer: AnyObject, Return: RbObjectConvertible>(
+    public func defineMethod<SwiftPeer: AnyObject, Return: RbObjectConvertible & Sendable>(
                     _ name: String,
                     argsSpec: RbMethodArgsSpec = RbMethodArgsSpec(),
                     method: @escaping RbBoundMethodCallback<SwiftPeer, Return>) throws {

Tests/RubyGatewayTests/TestClassDef.swift

@@ -86,7 +86,7 @@ class TestClassDef: XCTestCase {
 
             let parentClass = try Ruby.get("MyParentClass")
             let myClass = try Ruby.defineClass("MyClass", parent: parentClass, under: innerMod)
-            var called = false
+            nonisolated(unsafe) var called = false
             try myClass.defineMethod("value") { _, _ in
                 called = true
                 return RbObject(100)
@@ -122,7 +122,7 @@ class TestClassDef: XCTestCase {
 
     // Bound Swift classes
 
-    class MyBoundClass {
+    class MyBoundClass: @unchecked Sendable {
 
         static nonisolated(unsafe) var initCount = 0
         static nonisolated(unsafe) var deinitCount = 0
@@ -139,7 +139,7 @@ class TestClassDef: XCTestCase {
         var fingerprint = MyBoundClass.fingerprintValue
 
         func getFingerprint(method: RbMethod) throws -> RbObject {
-            return RbObject(fingerprint)
+            RbObject(fingerprint)
         }
 
         var generation: Int

Tests/RubyGatewayTests/TestGlobalVars.swift

@@ -17,7 +17,7 @@ class TestGlobalVars: XCTestCase {
             let initialValue = 22
             let newValue = 108
 
-            var modelValue = initialValue
+            nonisolated(unsafe) var modelValue = initialValue
 
             let gvarName = "$myVirtualGlobal"
 
@@ -46,7 +46,7 @@ class TestGlobalVars: XCTestCase {
             let initialIntValue = 100
             let targetStringValue = "Berry"
 
-            var wrappedObj = RbObject(initialIntValue)
+            nonisolated(unsafe) var wrappedObj = RbObject(initialIntValue)
             let gvarName = "$myGlobal"
 
             try Ruby.defineGlobalVar(gvarName,

Tests/RubyGatewayTests/TestMethods.swift

@@ -43,7 +43,7 @@ class TestMethods: XCTestCase {
             let argCount = 1
             let argValue = "Fish"
             let retValue = 8.9
-            var visited = false
+            nonisolated(unsafe) var visited = false
 
             try Ruby.defineGlobalFunction(funcName, argsSpec: RbMethodArgsSpec(leadingMandatoryCount: argCount)) { _, method in
                 method.checkArgs()
@@ -65,7 +65,7 @@ class TestMethods: XCTestCase {
         doErrorFree {
             let funcName = "myGlobal"
             let retValue = 8.9
-            var visited = false
+            nonisolated(unsafe) var visited = false
 
             try Ruby.defineGlobalFunction(funcName) { _, method in
                 method.checkArgs()
@@ -104,7 +104,7 @@ class TestMethods: XCTestCase {
     func testGoodBlock() {
         doErrorFree {
             let funcName = "myGlobal"
-            var funcCalled = false
+            nonisolated(unsafe) var funcCalled = false
             var blockCalled = false
             let expectedBlockResult = 4.0
             let expectedFuncResult = "alldone"
@@ -140,7 +140,7 @@ class TestMethods: XCTestCase {
     func testErrorNoBlock() {
         doErrorFree {
             let funcName = "myGlobal"
-            var funcCalled = false
+            nonisolated(unsafe) var funcCalled = false
 
             try Ruby.defineGlobalFunction(funcName) { _, method in
                 funcCalled = true
@@ -159,8 +159,8 @@ class TestMethods: XCTestCase {
     func testManualBlock() {
         doErrorFree {
             let funcName = "myGlobal"
-            var funcCalled = false
-            var blockCalled = false
+            nonisolated(unsafe) var funcCalled = false
+            nonisolated(unsafe) var blockCalled = false
 
             try Ruby.defineGlobalFunction(funcName) { _, method in
                 let block = try method.captureBlock()
@@ -184,8 +184,8 @@ class TestMethods: XCTestCase {
     func testBlockArgs() {
         doErrorFree {
             let funcName = "myGlobal"
-            var funcCalled = false
-            var blockCalled = false
+            nonisolated(unsafe) var funcCalled = false
+            nonisolated(unsafe) var blockCalled = false
             let expectedBlockArg = 4.0
 
             try Ruby.defineGlobalFunction(funcName, argsSpec: RbMethodArgsSpec(requiresBlock: true)) { _, method in
@@ -284,7 +284,7 @@ class TestMethods: XCTestCase {
             // def f(a=3, b=4)
             let spec_f = RbMethodArgsSpec(optionalValues: [3, 4])
             let func_f = "f"
-            var expectedArgs_f: [RbObject] = []
+            nonisolated(unsafe) var expectedArgs_f: [RbObject] = []
             try Ruby.defineGlobalFunction(func_f, argsSpec: spec_f) { _, method in
                 method.checkArgs()
                 XCTAssertEqual(expectedArgs_f, method.args.optional)
@@ -311,7 +311,7 @@ class TestMethods: XCTestCase {
             // def f(*a)
             let spec_f = RbMethodArgsSpec(supportsSplat: true)
             let func_f = "f"
-            var expectedArgs_f: [RbObject] = []
+            nonisolated(unsafe) var expectedArgs_f: [RbObject] = []
             try Ruby.defineGlobalFunction(func_f, argsSpec: spec_f) { _, method in
                 method.checkArgs()
                 XCTAssertEqual(expectedArgs_f, method.args.splatted)
@@ -337,9 +337,9 @@ class TestMethods: XCTestCase {
                                           supportsSplat: true,
                                           trailingMandatoryCount: 1)
             let func_f = "f"
-            var a_val: RbObject = .nilObject
-            var c_val: RbObject = .nilObject
-            var b_count: Int = 0
+            nonisolated(unsafe) var a_val: RbObject = .nilObject
+            nonisolated(unsafe) var c_val: RbObject = .nilObject
+            nonisolated(unsafe) var b_count: Int = 0
             try Ruby.defineGlobalFunction(func_f, argsSpec: spec_f) { _, method in
                 method.checkArgs()
                 XCTAssertEqual(a_val, method.args.mandatory[0])
@@ -398,7 +398,7 @@ class TestMethods: XCTestCase {
             // def f(ar:)
             let spec_f = RbMethodArgsSpec(mandatoryKeywords: [argKey])
             let expectedVal = 211.4896
-            var called = false
+            nonisolated(unsafe) var called = false
 
             try Ruby.defineGlobalFunction(func_f, argsSpec: spec_f) { _, method in
                 method.checkArgs()
@@ -462,7 +462,7 @@ class TestMethods: XCTestCase {
             let f_opt_arg_def = "fish"
             let spec_f = RbMethodArgsSpec(optionalKeywordValues: [f_opt_arg_kw: f_opt_arg_def])
 
-            var expect_arg_val = ""
+            nonisolated(unsafe) var expect_arg_val = ""
             try Ruby.defineGlobalFunction(func_f, argsSpec: spec_f) { _, method in
                 method.checkArgs()
                 XCTAssertEqual(expect_arg_val, String(method.args.keyword[f_opt_arg_kw]!))
@@ -490,8 +490,8 @@ class TestMethods: XCTestCase {
                                           optionalValues: [b_def],
                                           optionalKeywordValues: [kw_c: kw_c_def])
 
-            var expect_a = ""
-            var expect_b = RbObject(b_def)
+            nonisolated(unsafe) var expect_a = ""
+            nonisolated(unsafe) var expect_b = RbObject(b_def)
 
             try Ruby.defineGlobalFunction(func_f, argsSpec: spec_f) { _, method in
                 method.checkArgs()

Tests/RubyGatewayTests/TestObjMethods.swift

@@ -18,8 +18,7 @@ class TestObjMethods: XCTestCase {
         doErrorFree {
             try Ruby.require(filename: Helpers.fixturePath("swift_obj_methods.rb"))
 
-
-            var callCount = 0
+            nonisolated(unsafe) var callCount = 0
 
             let clazz = try Ruby.get("EmptyClass")
 
@@ -68,7 +67,7 @@ class TestObjMethods: XCTestCase {
         doErrorFree {
             try Ruby.require(filename: Helpers.fixturePath("swift_obj_methods.rb"))
 
-            var called = false
+            nonisolated(unsafe) var called = false
 
             let module = try Ruby.get("EmptyModule")
             XCTAssertEqual(RbType.T_MODULE, module.rubyType)
@@ -88,7 +87,7 @@ class TestObjMethods: XCTestCase {
         doErrorFree {
             try Ruby.require(filename: Helpers.fixturePath("swift_obj_methods.rb"))
 
-            var callCount = 0
+            nonisolated(unsafe) var callCount = 0
 
             let clazz = try Ruby.get("IdentifiedClass")
             try clazz.defineMethod("doubleId") { rbSelf, method in
@@ -107,7 +106,7 @@ class TestObjMethods: XCTestCase {
         doErrorFree {
             try Ruby.require(filename: Helpers.fixturePath("swift_obj_methods.rb"))
 
-            var callCount = 0
+            nonisolated(unsafe) var callCount = 0
 
             let clazz = try Ruby.get("BaseClass")
             try clazz.defineMethod("getValue") { rbSelf, method in
@@ -125,7 +124,7 @@ class TestObjMethods: XCTestCase {
         doErrorFree {
             try Ruby.require(filename: Helpers.fixturePath("swift_obj_methods.rb"))
 
-            var callCount = 0
+            nonisolated(unsafe) var callCount = 0
 
             let clazz = try Ruby.get("OverriddenClass")
             try clazz.defineMethod("getValue") { rbSelf, method in
@@ -158,7 +157,7 @@ class TestObjMethods: XCTestCase {
     func testSingleton() {
         doErrorFree {
             let module = try Ruby.get("Math")
-            var called = false
+            nonisolated(unsafe) var called = false
             try module.defineSingletonMethod("double", argsSpec: .basic(1)) { _, method in
                 called = true
                 return method.args.mandatory[0] * 2
@@ -207,7 +206,7 @@ class TestObjMethods: XCTestCase {
         doErrorFree {
             try Ruby.require(filename: Helpers.fixturePath("swift_obj_methods.rb"))
 
-            var called = false
+            nonisolated(unsafe) var called = false
 
             let clazz = try Ruby.get("SingBase")
             try clazz.defineSingletonMethod("value2") { rbSelf, _ in