swift-bridge icon indicating copy to clipboard operation
swift-bridge copied to clipboard

Implementing Swift's Error and Sendable protocols on an opaque Rust type's generated Swift class

Open chinedufn opened this issue 2 years ago • 26 comments

The Problem

In #149 we implemented support for returning -> Result<T, E> where E is an opaque Rust type.

// Rust

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        type TypeA;
        type TypeB;

        fn some_function() -> Result<TypeA, TypeB>;

        fn print_info(self: &TypeB);
    }
}

Swift errors need to conform to Swift's Error and Sendable protocols, but right now there is no way to declare that an Opaque Rust type should conform to Error and Sendable.

Instead, you currently have to manually specify that a type conforms.

https://github.com/chinedufn/swift-bridge/blob/2ab37cbb949be4a466848129e7f17ecbb3fecf5e/SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunner/Result.swift#L26-L27

An Example "Solution"

NOTE: This is a quick sketch of a potential solution in order to help make the problem more clear. The concurrency implications have not been deeply considered. Before working on this we should take a step back to lay out and consider all of the Rust + Swift concurrency factors at play. The final solution might not look anything like this sketch. The "Resources" section below links to resources that we will want to study and reference while figuring out our design.

If an OpaqueRustType implements Rust's Send and Sync traits we may want the user to be able to emit a Sendable protocol implementation.

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        // If the type does not implement Rust's `Send + Sync` traits we get a
        // compile time error
        #[swift_bridge(Sendable)]
        type TypeA;
    }
}

// This implements `Send` and `Sync`, so
// the `Sendable` attribute above works.
struct TypeA {
    field: Vec<u8>
}

On the Swift side this could generate something like:

// Swift

extension TypeA: @unchecked Sendable {}
extension TypeA: Error {}

NOTE: The above is a quick sketch of a potential solution in order to help make the problem more clear. The concurrency implications have not been deeply considered. Before working on this we should take a step back to lay out and consider all of the Rust + Swift concurrency factors at play. The final solution might not look anything like this sketch. The "Resources" section below links to resources that we will want to study and reference while figuring out our design.

Considerations

Sendable

We need to have a good understanding of Swift's Sendable protocol's requirements.

The Resources section below contains links with more information.

Swift reference types

The struct TypeA in Rust becomes a class TypeA in Swift.

If this class TypeA implements Sendable then it can be sent across threads, meaning that two different threads could access the mutable methods on the instance of class TypeA at the same time.

Is it possible for us to prevent this? Could Swift's Actors be of use here?

Concurrency

In general we need to think through Swift's concurrency model and make sure that we are going down a safe path.

Getting some design feedback from someone with a good amount of Swift experience would be helpful.

Resources

chinedufn avatar Jan 29 '23 01:01 chinedufn

Please add https://docs.swift.org/swift-book/LanguageGuide/Concurrency.html to Resources.

NiwakaDev avatar Jan 29 '23 11:01 NiwakaDev

First of all, how about supporting Swift's Sendable protocol when fields of Rust's struct consist of only primitive types such as bool?

NiwakaDev avatar Jan 30 '23 23:01 NiwakaDev

Do you mean opaque Rust types or transparent shared structs? Can you comment with a bridge module illustrating what you mean?

chinedufn avatar Jan 30 '23 23:01 chinedufn

Do you mean opaque Rust types or transparent shared structs?

I meant opaque Rust types.

Can you comment with a bridge module illustrating what you mean?


#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        #[swift_bridge(Sendable)]
        type TypeA;
    }
}

struct TypeA {
    field: i32
}

//Swift

//...
extension TypeA: @unchecked Sendable {}
extension TypeA: Error {}

TypeA conforms to Sendable with @unchecked. But, this can obviously conform to Sendable without it because this fields consists of only primitive types. So, this might be a good starting point.

Edit: If a Opaque Rust Type has has some methods with &mut self, my opinion is wrong.

NiwakaDev avatar Mar 19 '23 16:03 NiwakaDev

Could Swift's Actors be of use here?

I don't think so. Swift's Actors is related to Swift's Task. Swift's Actor can be accessed by only one Swift's Task at a time. This prevents data-race. But, if we use Task, we need to think through what happens when calling Rust methods within Task. This will absolutely complicate swift-bridge.

NiwakaDev avatar Mar 19 '23 16:03 NiwakaDev

If Opaque Rust Types have some methods with &mut self, we need to use Arc<Mutex<T>> or something like that, like so.


impl SomeType {
    fn some_method(&mut self) {
        self.0.lock().unwrap();
        //...
    }
}

Maybe, we need to have SomeTypeWrapper, like so:

impl SomeTypeWrapper {
    fn some_method(&mut self) {
        self.0.lock().unwrap();
        self.0.some_method();
        //...
    }
}

NiwakaDev avatar Mar 19 '23 17:03 NiwakaDev

Ok, I think I have a potential solution in mind.

Here are some thoughts:


Imagine the following bridge module:

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        type MySendSyncType;

        fn owned(self);
        fn ref(&self);
        fn ref_mut(&mut self);
    }
}

struct MySendSyncType {
    field: u32
}

Today, this generates the following Swift classes:

class MySendSyncType: MySendSyncTypeRefMut {
    // ...

    func owned() { /* */ }
}
class MySendSyncTypeRefMut: MySendSyncTypeRef {
    // ...

    func ref_mut() { /* */ }
}
class MySendSyncTypeRef {
    // ...

    func ref() { /* */ }
}

class MySendSyncType CANNOT be safely shared between Swift threads, since two threads could call .ref_mut() at the same time.

class MySendSyncTypeRef CAN be safely passed between Swift threads.

class MySendSyncTypeRefMut CANNOT be safely shared across Swift threads, since two threads could call .ref_mut() at the same time.


So, the requirements are:

  • Type is Send + Sync on the Rust side

  • Type has no mutable methods

So

  • MySendSyncType can be Swift Sendable if it has no mutable methods.

  • MySendSyncTypeRef can always be Swift Sendable.

  • MySendSyncTypeRefMut can always be Swift Sendable if it has no mutable methods.


One problem, though, is that the bridge macro can't know whether or not an opaque Rust type exposes mutable methods, since its possible to split a type's methods across multiple bridge modules.

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        type MySendSyncType;

        fn owned(self);
        fn ref(&self);
    }
}

#[swift_bridge::bridge]
mod ffi2 {
    extern "Rust" {
        #[swift_bridge(already_declared)]
        type MySendSyncType;

        fn ref_mut(&mut self);
    }   
}

Alright, so we know that MySendSyncTypeRef can always be made Sendable, but that isn't that great since users would then need to make sure that they remember not to try to use it after the reference's lifetime.

In most cases you'll want a Sendable owned MySendSyncType.

So, the question becomes, how can we guarantee that a bridged Rust type does not have any mutable methods?

Even if it has method declarations across multiple bridge modules?

Potential Solution 1 - Have swift-bridge-build keep track of information across multiple bridge modules

One approach would be to have swift-bridge-build look for types that are Sendable.

Right now swift-bridge-build iterates over all of your bridge files, parses the bridge modules, generates the Swift and C code for each module and then concatenates it all together into a final Swift and C file.

https://github.com/chinedufn/swift-bridge/blob/23d60feaa0f8e92f2c854d8665868aa5573ef505/crates/swift-bridge-build/src/lib.rs#L16-L45

We could make this process instead first combine all of the bridge modules into one data structure and then generate the Swift and C code from that data structure.

In this way we could know whether or not we can make MySendSyncType and MySendSyncTypeRefMut Sendable, since we know whether or not it has any mutable methods.

Downsides

  • You won't get a compile time error while you write your bridge macro. You'll only get the error when you try to use swift-bridge-build.

  • ... think through other downsides ...

Potential Solution 2 - Don't allow mutable methods on Sendable opaque Rust types

We could have something like:

// In swift_bridge/src/std_bridge/sendable.rs

trait SwiftSendable: Send + Sync {}
trait NotSwiftSendable {}

fn assert_sendable<T: SwiftSendable>() {}
fn assert_not_sendable<T: NotSwiftSendable>() {}

Then for the following bridge module:

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        #[swift_bridge(Sendable)]
        type MySendSyncType;

        fn owned(self);
        fn ref(&self);
    }
}

We would emit:

mod ffi {
    // ...
    impl swift_bridge::sendable::SwiftSendable for super::MySendSyncType {}
}

And if a module uses already_declared

#[swift_bridge::bridge]
mod ffi2 {
    extern "Rust" {
        #[swift_bridge(already_declared)]
        type MySendSyncType;

        #[swift_bridge(already_declared, Sendable)]
        type AnotherSendSyncType;

        fn ref_mut(&mut self);
    }   
}

It emits something like:

mod ffi2 {
    // ...

    swift_bridge::sendable::assert_sendable<super::AnotherSendSyncType>();
    swift_bridge::sendable::assert_not_sendable<super::MySendSyncType>();
}

This way we know for sure that Sendable was used across all all extern Rust type MySendSyncType declarations. Otherwise there would be a compile time error where we incorrectly asserted that something was SwiftSendable or NotSwiftSendable.

If we see any mutable methods on a Sendable type inside of any bridge module type we would emit a compile time error.

This is nice because we don't end up in situations where a user marks something as swift_bridge(Sendable) but only the Ref type ends up being Sendable. That would be confusing.


If you need mutability, you can use interior mutabiliy

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        #[swift_bridge(Sendable)]
        type MyType;

        fn increment(&self);
    }
}

struct MyType {
    counter: Arc<Mutex<u32>>
}
impl MyType {
    pub fn increment(&self) {
        *self.counter.lock().unwrap() += 1;
    }
}

Downsides

  • You lose the ability to have MySendSyncTypeRef still be Sendable when MySendSyncType and MySendSyncTypeRefMut cannot be Sendable.
    • This doesn't sound useful to me in practice. Having the Ref be Sendable but not the owned type would cause confusion.

Conclusion

These are some quick thoughts that I just wrote down and haven't thought through too deeply.

Wanted to share them for discussion.

Right now I'm leaning towards Potential Solution 2 - Don't allow mutable methods on Sendable opaque Rust types.

I like that it forces all bridge modules to explicitly declare already_declared types as Sendable, so that if you're reading any bridge module you know whether or not the type is Sendable.


Thoughts?

chinedufn avatar Mar 19 '23 19:03 chinedufn

Thank you for your quick reply.

Thoughts?

I'll read it this weekend!!

By the way, right now we can use multithreading on both Swift and Rust.

So, I have a question about:

isOwned is thread-safe?

https://github.com/chinedufn/swift-bridge/blob/b654a24d6d5169e0e73192ebffa556cd43044512/crates/swift-bridge-build/src/generate_core/rust_string.swift#L2

NiwakaDev avatar Mar 22 '23 11:03 NiwakaDev

Hmm, that's a good question.

isOwned gets modified when

  • (case 1) the deinit method on a class OpaqueRustType instance is called.
  • (case 2) you pass on owned class OpaqueRustType from Swift -> Rust.

(case 1): If Swift calls deinit then there is exactly one instance remaining, so there can't be a data race there.


(case 2): After passing a class OpaqueRustType from Swift to Rust, any further usage on the Swift side is undefined behavior.

Two threads could both have an instance of OpaqueRustType and both try to pass ownership to Rust at the same time.

This is a problem.

Idea 1 - No passing ownership of Sendable Rust types from Swift -> Rust

One idea for how we might for this would be to make it impossible by not letting you pass owned Sendable Rust types from Swift -> Rust.

So, for example:

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        #[swift_bridge(Sendable)]
        type OpaqueRustType;

        // Compile time error since Swift isn't allowed to pass ownership back to Rust.
        fn take(self);

        // Compile time error since Swift isn't allowed to pass ownership back to Rust.
        fn eat(ty: OpaqueRustType);

        // This is fine since 
        fn by_ref(reference: &OpaqueRustType);
    }
}

~But, that isn't a great solution since you could still call .take() from one thread and .by_ref() from another thread which would be an issue.~ EDIT - actually that wouldn't be a problem since.. take wouldn't compile.

Idea 2 - Use synchronization for the isOwned if the type is Sendable

for a #[swift_bridge(Sendable)] opaque Rust type we could consider putting the isOwned Bools access behind some sort of Swift synchronization primitive.

Then we could have an optional check in every generated class method of an owned class instance that confirms that isOwned is true. If it's false, it panics.

class OpaqueRustType {
    var isOwned: SomeSynchronizedBool

    func take() {
        if !isOwned { fatalError() }

        // ...
    }

    func eat() {
        if !isOwned { fatalError() }

        // ...
    }

    // ...
}

Something like this might also be useful for non Sendable types since it would turn use after frees into defined behavior (fatal error).

I've had this check idea in the past, but I don't like that it adds overhead to every call. At the very least we might want to always enable something like this in debug builds. Then figure out whether it should be on or off by default in release builds.

Idea 3 - Do nothing

One option is to just allow this to be a problem that the user must manage and document it (unless there's some Swift feature that lets us solve this problem without trade-offs).

For example, this is related to the the Never use a value after its dropped safety rule in our book https://github.com/chinedufn/swift-bridge/blob/637c7b353539c1122dacc3a64e7d5dc052a4fec9/book/src/safety/README.md?plain=1#L122-L148 .

Conclusion

I haven't used swift-bridge in a multi threaded context so I don't have a good sense of the trade-offs on how we might go about handling thread safety.

So, I'm starting by just sharing the ideas above.

chinedufn avatar Mar 22 '23 22:03 chinedufn

Idea 3 - Do nothing One option is to just allow this to be a problem that the user must manage and document it (unless there's some Swift feature that lets us solve this problem without trade-offs).

Sounds good to me.

I've had this check idea in the past, but I don't like that it adds overhead to every call.

If a class Opaque Rust Type is Sendable, what about releasing its resoureces not on the Rust side but only on the Swift side? This means that its resources get released from only deinit.

So, we can remove isOwned from a class Opaque Rust Type that conforms that Sendable, like so:

class OpaqueRustType {
   //var isOwned: Bool 
}

This advantage. : we don't need think whether or not isOwned is thread-safe, and there is no overhead. This disadvantage : we can't have owned methods if a class Opaque Rust Type is Sendable.

I haven't used swift-bridge in a multi threaded context so I don't have a good sense of the trade-offs on how we might go about handling thread safety.

Me either! But, if swift-bridgesupports Sendable, we need to think through handling isOwned, and then I guess that we could finally start to discuss supporting Sendable.

NiwakaDev avatar Mar 23 '23 03:03 NiwakaDev

This means that its resources get released from only deinit

I think that we'll eventually want to be able to explicitly pass ownership of Sendable opaque Rust types from Swift -> Rust, to support use cases such as being able to pass an opaque Rust type to a worker thread and then call some owned method on it to clean it up immediately after processing the job, but we don't need support for this from the outset.

So, I like it.

So, we can remove isOwned from a class Opaque Rust Type that conforms that Sendable, like so:

Oh nice great idea.


Long term I like Idea 2 - Use synchronization for the isOwned if the type is Sendable since it would give the developer the ability to safely transfer ownership from Swift -> Rust, but as a starting point I think that only freeing from deinit is great since I'm not sure that needing to pass ownership back to Rust is all that common.


I'm imagining that the compile time error on functions that pass ownership from Swift -> Rust such as:

mod ffi {
    extern "Rust" {
        #[swift_bridge(Sendable)]
        type OpaqueRustType;

        // Compile time error since Swift isn't allowed to pass ownership back to Rust.
        fn take(self);

        // Compile time error since Swift isn't allowed to pass ownership back to Rust.
        fn eat(ty: OpaqueRustType);
    }

    extern "Swift" {
        // Compile time error since Swift isn't allowed to pass ownership back to Rust.
        fn x() -> OpaqueRustType;
    }
}

could have a compile time error like:

Passing ownership of `Sendable` opaque Rust types from `Swift` to `Rust` is not yet supported.
Please let us know about your use case in https://github.com/chinedufn/swift-bridge/issues/150

Then when people run into this they can give us more context around their needs and we can use that to design the right solution (i.e. maybe Idea 2 - Use synchronization for the isOwned if the type is Sendable, or something else).

chinedufn avatar Mar 23 '23 17:03 chinedufn

I'm imagining that the compile time error on functions that pass ownership from Swift -> Rust such as:

Sounds good to me!!

Long term I like Idea 2 - Use synchronization for the isOwned if the type is Sendable since it would give the developer the ability to safely transfer ownership from Swift -> Rust, but as a starting point I think that only freeing from deinit is great since I'm not sure that needing to pass ownership back to Rust is all that common.

I think so too. We can start to discuss supporting Sendable based on Idea 2.

NiwakaDev avatar Mar 23 '23 22:03 NiwakaDev

Hi @chinedufn.

Based on the previous discussion,

To make the OpaqueRustType that conforms to Sendable:

  1. Needs to conform to Sync+Send on the Rust side.
  2. To release its resources from only Swift deinit, it must have only immutable methods.

Something like:


#[swift_bridge]
mod ffi {
   extern "Rust" {
      #[swift_bridge(Sendable)]
      type SomeType;
      fn immutable_method(&self)//We only have immutable methods.
   }
}

This should generate:

class SomeType {
    //var isOwned: Bool = true //this get deleted.

    public init(ptr: UnsafeMutableRawPointer) {
        super.init(ptr: ptr)
    }
    public func immutable_method() {

    }
    deinit {
        //Since we don't need to have the isOwned, so we could delete the if statement.
        //if isOwned {
            __swift_bridge__$SomeType$_free(ptr)
        //}
    }
}

How about this?

NiwakaDev avatar Apr 04 '23 11:04 NiwakaDev

Handling the same type declaration across multiple bridge modules

Even if it has method declarations across multiple bridge modules?

I agree with "Potential Solution 2 - Don't allow mutable methods on Sendable opaque Rust types".

You said :

This doesn't sound useful to me in practice. Having the Ref be Sendable but not the owned type would cause confusion.

If we adopt the idea immediately above, we don't need to care about this since we couldn't have owned methods

NiwakaDev avatar Apr 04 '23 11:04 NiwakaDev

By the way, I guess that a Sendable OpaqueRustType couldn't have mutable arguments and owned arguments, like so:


#[swift_bridge]
mod ffi {
   extern "Rust" {
      type SomeType2;
   }
   extern "Rust" {
      #[swift_bridge(Sendable)]
      type SomeType;
      fn immutable_method(&self, some_value: &mut i32, some_type2: SomeType2)//This is not thread-safe??
   }
}

This might be a problem...Maybe, other arguments also need to conform to Sendable.... On the Rust side, they also need to implement Sync+Send.

NiwakaDev avatar Apr 04 '23 12:04 NiwakaDev

How about this?

Sounds about right to me!

If we adopt the idea immediately above, we don't need to care about this since we couldn't have owned methods

Agreed

By the way, I guess that a Sendable OpaqueRustType couldn't have mutable arguments and owned arguments, like so:

I don't think that this would be a problem.

Say you're in thread A. If you call immutable_method you're still in thread A on the Rust side. Rust can't pass SomeType2 to another thread unless it's thread safe. If it isn't thread safe Rust won't let you.

So, Sendable OpaqueRustType methods CAN have mutable arguments and owned arguments.


Thanks for exploring all of this!

chinedufn avatar Apr 06 '23 01:04 chinedufn

I don't think that this would be a problem.

Let's say that we have two Tasks and have two OpaqueRustTypes on the Swift side, like so:

#[swift_bridge]
mod ffi {
   extern "Rust" {
      #[swift_bridge(Sendable)]
      type OpaqueRustType1(;
      fn immutable_method(&self, opaque_rust_type2: OpaqueRustType2)

   extern "Rust" {
      //Not Sendable
      type OpaqueRustType2;   
   }

}
let opaqueRustType1 = OpaqueRustType1()
let opaqueRustType2 = OpaqueRustType2()

//Task1 
Task {
   opaqueRustType1.immutableMethod(opaqueRustType2)
}

//Task2
Task {
   opaqueRustType1.immutableMethod(opaqueRustType2)
}

So, I guess that the above code snippet might not be thread-safe.

NiwakaDev avatar Apr 08 '23 07:04 NiwakaDev

Sorry I'm not very familiar with Swift Tasks. If all of that code is running on the same thread then it looks thread safe to me.

Why wouldn't this be thread safe?

Can you move a Task to a different thread if it captures non Sendable values?

chinedufn avatar Apr 10 '23 17:04 chinedufn

Can you move a Task to a different thread if it captures non Sendable values?

Yes. But, Swift6 should generate any compile errors when a Task captures non Sendable values. Right now Swift version is 5.8.

NiwakaDev avatar Apr 12 '23 10:04 NiwakaDev

Nice thanks for researching this.


So, even if we tried to prevent methods that take owned arguments a user could always do:

let opaqueRustType2 = OpaqueRustType2()

//Task1 
Task {
    opaqueRustType2.some_owned_method()
}

//Task2
Task {
    opaqueRustType2.some_owned_method()
}

If the problem will entirely disappear in Swift 6 I don't think that we should try to address it. We can just add a section about thread safety to the Safety chapter in the book that shows this problem with Sendable opaque Rust types and lets readers know that it will go away in Swift 6.

Then when Swift 6+ is ubiquitous we can delete this from the safety docs.

chinedufn avatar Apr 13 '23 11:04 chinedufn

@chinedufn

This issue should be separated into multiple tasks:

  • [ ] Supports Sendable for OpaqueRustType
  • [ ] Supports Sendable for OpaqueRustType across multiple bridge modules

I'd like to work on the first task. What do you think about this?

NiwakaDev avatar Jul 15 '23 10:07 NiwakaDev

Sounds good!

chinedufn avatar Jul 15 '23 16:07 chinedufn

For Rust opaque types that implement std::error::Error, it would be nice to be able to automatically generate a Swift Error extension using the type's Display implementation. All that would be needed is for the type to have the #[swift_bridge(Error)] attribute, then a function would be generated to call the type's to_string function, which the generated Swift extension would use for localizedDescription.

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        #[swift_bridge(Error)] // Causes FFI function to be generated for `self.to_string()` and something like `extension SomeError: Error { var localizedDescription: String { swift_bridge_SomeError_internal_to_string(self) }`
        type SomeError;
    }
}

#[derive(thiserror::Error)]
pub enum SomeError {
    #[error("Error message")]
    Error,
}

It might be nice to allow this attribute on normal functions:

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        type SomeError;
        #[swift_bridge(Error)]
        fn display(&self) -> String; // Allows the user to customize the string used for localizedDescription, for example using the Debug implementation instead (this could just be an option on the attribute)
    }
}

If this seems like a good solution I may be able to implement it. (not sure about the Sendable stuff; I've been hand writing Error extensions for Rust types and never had any issues)

amsam0 avatar Aug 20 '23 03:08 amsam0

A Swift Error must implement Sendable, so I'm not sure that we would want #[swift_bridge(Error)] support without nailing down our send/sync story.

It looks like the localizedDescription is available on Swift Errors that implement the Swift LocalizedError protocol

https://stackoverflow.com/a/39176551 https://developer.apple.com/documentation/foundation/localizederror

So, maybe we could have a #[swift_bridge(LocalizedError)] attribute that generates a LocalizedError protocol implementation?

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        #[swift_bridge(LocalizedError)]
        type SomeError;

        fn error_description(&self) -> String;
    }
}

chinedufn avatar Aug 20 '23 20:08 chinedufn

If a opaque type implements Error, the opaque type can't have owned methods for now.

NiwakaDev avatar Aug 20 '23 22:08 NiwakaDev

It looks like the localizedDescription is available on Swift Errors that implement the Swift LocalizedError protocol

Normal Errors also seem to have this property: https://developer.apple.com/documentation/swift/error/localizeddescription

Is Sendable only automatically implemented for transparent types? That would explain why I've been able to hand write Error extensions.

amsam0 avatar Aug 21 '23 14:08 amsam0