diff --git a/3rd-party/uniffi-rs b/3rd-party/uniffi-rs index cd38cce..149a51d 160000 --- a/3rd-party/uniffi-rs +++ b/3rd-party/uniffi-rs @@ -1 +1 @@ -Subproject commit cd38ccea8236df7d93aff336c325a3a8e524af5d +Subproject commit 149a51d034711d74c50296f1d027f66926b9fdd9 diff --git a/Cargo.lock b/Cargo.lock index a89e3ef..8a43244 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -973,7 +973,7 @@ checksum = "c0edd1e5b14653f783770bce4a4dabb4a5108a5370a5f5d8cfe8710c361f6c8b" [[package]] name = "uniffi" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "camino", @@ -1171,7 +1171,7 @@ dependencies = [ [[package]] name = "uniffi_bindgen" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "askama 0.12.1", @@ -1193,7 +1193,7 @@ dependencies = [ [[package]] name = "uniffi_build" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "camino", @@ -1202,7 +1202,7 @@ dependencies = [ [[package]] name = "uniffi_checksum_derive" -version = "0.28.0" +version = "0.3.1" dependencies = [ "quote", "syn 2.0.37", @@ -1210,7 +1210,7 @@ dependencies = [ [[package]] name = "uniffi_core" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "async-compat", @@ -1224,7 +1224,7 @@ dependencies = [ [[package]] name = "uniffi_macros" -version = "0.28.0" +version = "0.3.1" dependencies = [ "bincode", "camino", @@ -1241,7 +1241,7 @@ dependencies = [ [[package]] name = "uniffi_meta" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "bytes", @@ -1251,7 +1251,7 @@ dependencies = [ [[package]] name = "uniffi_testing" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "camino", @@ -1262,7 +1262,7 @@ dependencies = [ [[package]] name = "uniffi_udl" -version = "0.28.0" +version = "0.3.1" dependencies = [ "anyhow", "textwrap 0.16.0", diff --git a/bindgen/src/gen_cs/mod.rs b/bindgen/src/gen_cs/mod.rs index a9c703e..cf0a2a8 100644 --- a/bindgen/src/gen_cs/mod.rs +++ b/bindgen/src/gen_cs/mod.rs @@ -367,7 +367,7 @@ impl CsCodeOracle { FfiType::UInt8 => "byte".to_string(), FfiType::Float32 => "float".to_string(), FfiType::Float64 => "double".to_string(), - FfiType::RustArcPtr(name) => format!("{}SafeHandle", name), + FfiType::RustArcPtr(_) => "IntPtr".to_string(), FfiType::RustBuffer(_) => "RustBuffer".to_string(), FfiType::ForeignBytes => "ForeignBytes".to_string(), FfiType::Callback(name) => self.ffi_callback_name(name), diff --git a/bindgen/templates/ObjectRuntime.cs b/bindgen/templates/ObjectRuntime.cs index cb081bf..08d1cd6 100644 --- a/bindgen/templates/ObjectRuntime.cs +++ b/bindgen/templates/ObjectRuntime.cs @@ -6,6 +6,16 @@ // method will only be called once, once all outstanding native calls have completed. // https://github.com/mozilla/uniffi-rs/blob/0dc031132d9493ca812c3af6e7dd60ad2ea95bf0/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt#L31 // https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.criticalhandle +// +// SafeHandle is used to manage ownership of the Rust object. When SafeHandle goes out of scope, +// the underlying Rust reference is decremented. Method calls and lowering increment ("clone") +// object reference, and Rust is responsible for decrementing the reference. This means, it's not +// necessary to use SafeHandle directly for method calls and lowering, since Rust will take care +// of decrementing the reference. +// +// However, it's still necessary to use SafeHandle for "cloning" the reference, since CG may choose +// to collect the object itself and run the finalizer, in turn decrementing Rust reference and +// causing use-after-free during "clone" native call. (See SafeHandle documentation). {{ config.access_modifier() }} abstract class FFIObject: IDisposable where THandle : FFISafeHandle { private THandle handle; @@ -36,18 +46,6 @@ public override bool IsInvalid { return handle.ToInt64() == 0; } } - - // TODO(CS) this completely breaks any guarantees offered by SafeHandle.. Extracting - // raw value from SafeHandle puts responsiblity on the consumer of this function to - // ensure that SafeHandle outlives the stream, and anyone who might have read the raw - // value from the stream and are holding onto it. Otherwise, the result might be a use - // after free, or free while method calls are still in flight. - // - // This is also relevant for Kotlin. - // - public IntPtr DangerousGetRawFfiValue() { - return handle; - } } static class FFIObjectUtil { diff --git a/bindgen/templates/ObjectTemplate.cs b/bindgen/templates/ObjectTemplate.cs index 966bf9d..67b6222 100644 --- a/bindgen/templates/ObjectTemplate.cs +++ b/bindgen/templates/ObjectTemplate.cs @@ -30,7 +30,7 @@ override protected bool ReleaseHandle() { {%- call cs::docstring(obj, 0) %} {{ config.access_modifier() }} class {{ type_name }}: FFIObject<{{ safe_handle_type }}>, I{{ type_name }} { - public {{ type_name }}({{ safe_handle_type }} pointer): base(pointer) {} + public {{ type_name }}(IntPtr pointer): base(new {{ safe_handle_type }}(pointer)) {} {%- match obj.primary_constructor() %} {%- when Some with (cons) %} @@ -47,12 +47,12 @@ override protected bool ReleaseHandle() { {%- when Some with (return_type) %} public {{ return_type|type_name(ci) }} {{ meth.name()|fn_name }}({% call cs::arg_list_decl(meth) %}) { - return {{ return_type|lift_fn }}({%- call cs::to_ffi_call_with_prefix("this.GetHandle()", meth) %}); + return {{ return_type|lift_fn }}({%- call cs::to_ffi_call_with_prefix("this._uniffiClonePointer()", meth) %}); } {%- when None %} public void {{ meth.name()|fn_name }}({% call cs::arg_list_decl(meth) %}) { - {%- call cs::to_ffi_call_with_prefix("this.GetHandle()", meth) %}; + {%- call cs::to_ffi_call_with_prefix("this._uniffiClonePointer()", meth) %}; } {% endmatch %} {% endfor %} @@ -61,7 +61,7 @@ override protected bool ReleaseHandle() { {%- match tm %} {%- when UniffiTrait::Display { fmt } %} public override string ToString() { - return {{ Type::String.borrow()|lift_fn }}({%- call cs::to_ffi_call_with_prefix("this.GetHandle()", fmt) %}); + return {{ Type::String.borrow()|lift_fn }}({%- call cs::to_ffi_call_with_prefix("this._uniffiClonePointer()", fmt) %}); } {%- else %} {%- endmatch %} @@ -76,21 +76,27 @@ public override string ToString() { } {% endfor %} {% endif %} + + public IntPtr _uniffiClonePointer() { + return _UniffiHelpers.RustCall((ref UniffiRustCallStatus status) => { + return _UniFFILib.{{ obj.ffi_object_clone().name() }}(this.GetHandle(), ref status); + }); + } } -class {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, {{ safe_handle_type }}> { +class {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, IntPtr> { public static {{ obj|ffi_converter_name }} INSTANCE = new {{ obj|ffi_converter_name }}(); - public override {{ safe_handle_type }} Lower({{ type_name }} value) { - return value.GetHandle(); + public override IntPtr Lower({{ type_name }} value) { + return value._uniffiClonePointer(); } - public override {{ type_name }} Lift({{ safe_handle_type }} value) { + public override {{ type_name }} Lift(IntPtr value) { return new {{ type_name }}(value); } public override {{ type_name }} Read(BigEndianStream stream) { - return Lift(new {{ safe_handle_type }}(new IntPtr(stream.ReadLong()))); + return Lift(new IntPtr(stream.ReadLong())); } public override int AllocationSize({{ type_name }} value) { @@ -98,6 +104,6 @@ public override int AllocationSize({{ type_name }} value) { } public override void Write({{ type_name }} value, BigEndianStream stream) { - stream.WriteLong(Lower(value).DangerousGetRawFfiValue().ToInt64()); + stream.WriteLong(Lower(value).ToInt64()); } } diff --git a/bindgen/templates/macros.cs b/bindgen/templates/macros.cs index 64b0e40..6174f3c 100644 --- a/bindgen/templates/macros.cs +++ b/bindgen/templates/macros.cs @@ -65,8 +65,8 @@ // Note unfiltered name but ffi_type_name filters. -#} {%- macro arg_list_ffi_decl(func) %} - {%- if func.is_object_free_function() %} - IntPtr ptr, + {%- if func.is_object_clone_function() %} + SafeHandle ptr, {%- if func.has_rust_call_status_arg() %}ref UniffiRustCallStatus _uniffi_out_err{% endif %} {%- else %} {%- call arg_list_ffi_decl_xx(func) %}