Skip to content

Commit

Permalink
Fix some more Swift concurrency issues
Browse files Browse the repository at this point in the history
Converted some more vars to lets.  Swift still complains about async
calls.  I believe the next step would be to make all UniFFI objects
conform to `Sendable`.

Split the `FfiType::Reference` variant into `Reference` and
`MutReference`.  This allows us to define some more variables using
`let` in swift.

See mozilla#2279.
  • Loading branch information
bendk committed Nov 1, 2024
1 parent 1c8dd50 commit 2b05c3d
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
### What's changed?

- Switching jinja template engine from askama to rinja.
-
### ⚠️ Breaking Changes for external bindings authors ⚠️

- Added the `FfiType::MutReference` variant.

## v0.28.2 (backend crates: v0.28.2) - (_2024-10-08_)

Expand Down
4 changes: 3 additions & 1 deletion uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,9 @@ impl KotlinCodeOracle {
FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(),
FfiType::Callback(name) => self.ffi_callback_name(name),
FfiType::Struct(name) => self.ffi_struct_name(name),
FfiType::Reference(inner) => self.ffi_type_label_by_reference(inner),
FfiType::Reference(inner) | FfiType::MutReference(inner) => {
self.ffi_type_label_by_reference(inner)
}
FfiType::VoidPointer => "Pointer".to_string(),
}
}
Expand Down
4 changes: 3 additions & 1 deletion uniffi_bindgen/src/bindings/python/gen_python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,9 @@ impl PythonCodeOracle {
FfiType::Callback(name) => self.ffi_callback_name(name),
FfiType::Struct(name) => self.ffi_struct_name(name),
// Pointer to an `asyncio.EventLoop` instance
FfiType::Reference(inner) => format!("ctypes.POINTER({})", self.ffi_type_label(inner)),
FfiType::Reference(inner) | FfiType::MutReference(inner) => {
format!("ctypes.POINTER({})", self.ffi_type_label(inner))
}
FfiType::VoidPointer => "ctypes.c_void_p".to_string(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ mod filters {
// definitions use references. Those FFI functions aren't actually used, so we just
// pick something that runs and makes some sense. Revisit this once the references
// are actually implemented.
FfiType::Reference(_) => ":pointer".to_string(),
FfiType::Reference(_) | FfiType::MutReference(_) => ":pointer".to_string(),
FfiType::VoidPointer => ":pointer".to_string(),
FfiType::Struct(_) => {
unimplemented!("Structs are not implemented")
Expand Down
8 changes: 7 additions & 1 deletion uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,9 @@ impl SwiftCodeOracle {
FfiType::Callback(name) => format!("@escaping {}", self.ffi_callback_name(name)),
FfiType::Struct(name) => self.ffi_struct_name(name),
FfiType::Reference(inner) => {
format!("UnsafePointer<{}>", self.ffi_type_label(inner))
}
FfiType::MutReference(inner) => {
format!("UnsafeMutablePointer<{}>", self.ffi_type_label(inner))
}
FfiType::VoidPointer => "UnsafeMutableRawPointer".into(),
Expand Down Expand Up @@ -708,7 +711,10 @@ pub mod filters {
format!("{} _Nonnull", SwiftCodeOracle.ffi_callback_name(name))
}
FfiType::Struct(name) => SwiftCodeOracle.ffi_struct_name(name),
FfiType::Reference(inner) => format!("{}* _Nonnull", header_ffi_type_name(inner)?),
FfiType::Reference(inner) => {
format!("const {}* _Nonnull", header_ffi_type_name(inner)?)
}
FfiType::MutReference(inner) => format!("{}* _Nonnull", header_ffi_type_name(inner)?),
FfiType::VoidPointer => "void* _Nonnull".into(),
})
}
Expand Down
2 changes: 1 addition & 1 deletion uniffi_bindgen/src/bindings/swift/templates/Async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private func uniffiTraitInterfaceCallAsyncWithError<T, E>(

// Borrow the callback handle map implementation to store foreign future handles
// TODO: consolidate the handle-map code (https://github.com/mozilla/uniffi-rs/pull/1823)
fileprivate var UNIFFI_FOREIGN_FUTURE_HANDLE_MAP = UniffiHandleMap<UniffiForeignFutureTask>()
fileprivate let UNIFFI_FOREIGN_FUTURE_HANDLE_MAP = UniffiHandleMap<UniffiForeignFutureTask>()

// Protocol for tasks that handle foreign futures.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ fileprivate struct {{ trait_impl }} {

// Create the VTable using a series of closures.
// Swift automatically converts these into C callback functions.
static var vtable: {{ vtable|ffi_type_name }} = {{ vtable|ffi_type_name }}(
//
// This creates 1-element array, since this seems to be the only way to construct a const
// pointer that we can pass to the Rust code.
static let vtable: [{{ vtable|ffi_type_name }}] = [{{ vtable|ffi_type_name }}(
{%- for (ffi_callback, meth) in vtable_methods %}
{{ meth.name()|fn_name }}: { (
{%- for arg in ffi_callback.arguments() %}
Expand Down Expand Up @@ -101,9 +104,9 @@ fileprivate struct {{ trait_impl }} {
print("Uniffi callback interface {{ name }}: handle missing in uniffiFree")
}
}
)
)]
}

private func {{ callback_init }}() {
{{ ffi_init_callback.name() }}(&{{ trait_impl }}.vtable)
{{ ffi_init_callback.name() }}({{ trait_impl }}.vtable)
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
@_documentation(visibility: private)
#endif
fileprivate struct {{ ffi_converter_name }} {
fileprivate static var handleMap = UniffiHandleMap<{{ type_name }}>()
fileprivate static let handleMap = UniffiHandleMap<{{ type_name }}>()
}

#if swift(>=5.8)
Expand Down
5 changes: 3 additions & 2 deletions uniffi_bindgen/src/bindings/swift/templates/HandleMap.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
fileprivate class UniffiHandleMap<T> {
private var map: [UInt64: T] = [:]
fileprivate final class UniffiHandleMap<T>: @unchecked Sendable {
// All mutation happens with this lock held, which is why we implement @unchecked Sendable.
private let lock = NSLock()
private var map: [UInt64: T] = [:]
private var currentHandle: UInt64 = 1

func insert(obj: T) -> UInt64 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ open class {{ impl_class_name }}:
#endif
public struct {{ ffi_converter_name }}: FfiConverter {
{%- if obj.has_callback_interface() %}
fileprivate static var handleMap = UniffiHandleMap<{{ type_name }}>()
fileprivate static let handleMap = UniffiHandleMap<{{ type_name }}>()
{%- endif %}

typealias FfiType = UnsafeMutableRawPointer
Expand Down
2 changes: 2 additions & 0 deletions uniffi_bindgen/src/bindings/swift/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ fn compile_swift_module<T: AsRef<OsStr>>(
command
.current_dir(out_dir)
.arg("-emit-module")
// TODO(2279): Fix concurrency issues and uncomment this
//.arg("-strict-concurrency=complete")
.arg("-module-name")
.arg(module_name)
.arg("-o")
Expand Down
6 changes: 4 additions & 2 deletions uniffi_bindgen/src/interface/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ pub fn method_ffi_callback(trait_name: &str, method: &Method, index: usize) -> F
arguments: iter::once(FfiArgument::new("uniffi_handle", FfiType::UInt64))
.chain(method.arguments().into_iter().map(Into::into))
.chain(iter::once(match method.return_type() {
Some(t) => FfiArgument::new("uniffi_out_return", FfiType::from(t).reference()),
Some(t) => {
FfiArgument::new("uniffi_out_return", FfiType::from(t).mut_reference())
}
None => FfiArgument::new("uniffi_out_return", FfiType::VoidPointer),
}))
.collect(),
Expand All @@ -175,7 +177,7 @@ pub fn method_ffi_callback(trait_name: &str, method: &Method, index: usize) -> F
FfiArgument::new("uniffi_callback_data", FfiType::UInt64),
FfiArgument::new(
"uniffi_out_return",
FfiType::Struct("ForeignFuture".to_owned()).reference(),
FfiType::Struct("ForeignFuture".to_owned()).mut_reference(),
),
])
.collect(),
Expand Down
8 changes: 7 additions & 1 deletion uniffi_bindgen/src/interface/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ pub enum FfiType {
/// These are used to pass objects across the FFI.
Handle,
RustCallStatus,
/// Pointer to an FfiType.
/// Const pointer to an FfiType.
Reference(Box<FfiType>),
/// Mutable pointer to an FfiType.
MutReference(Box<FfiType>),
/// Opaque pointer
VoidPointer,
}
Expand All @@ -69,6 +71,10 @@ impl FfiType {
FfiType::Reference(Box::new(self))
}

pub fn mut_reference(self) -> FfiType {
FfiType::MutReference(Box::new(self))
}

/// Unique name for an FFI return type
pub fn return_type_name(return_type: Option<&FfiType>) -> String {
match return_type {
Expand Down

0 comments on commit 2b05c3d

Please sign in to comment.