Skip to content

Commit

Permalink
Update objects ABI to match 0.28.0
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
arg0d committed Aug 2, 2024
1 parent a506194 commit b485b2f
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 35 deletions.
2 changes: 1 addition & 1 deletion 3rd-party/uniffi-rs
Submodule uniffi-rs updated 40 files
+31 −0 .github/workflows/tests.yml
+28 −685 CHANGELOG.md
+217 −80 Cargo.lock
+3 −1 Cargo.toml
+47 −65 README.md
+20 −19 docker/Dockerfile-build
+12 −5 docker/cargo-docker.sh
+39 −0 docs/release-process-nordsec.md
+17 −0 examples/custom-types/uniffi.toml
+1 −0 fixtures/callbacks/src/callbacks.udl
+5 −0 fixtures/callbacks/src/lib.rs
+23 −0 fixtures/coverall-upstream-compatibility/Cargo.toml
+7 −0 fixtures/coverall-upstream-compatibility/README.md
+7 −0 fixtures/coverall-upstream-compatibility/build.rs
+297 −0 fixtures/coverall-upstream-compatibility/src/coverall-upstream-compatibility.udl
+61 −0 fixtures/coverall-upstream-compatibility/src/ffi_buffer_scaffolding_test.rs
+609 −0 fixtures/coverall-upstream-compatibility/src/lib.rs
+225 −0 fixtures/coverall-upstream-compatibility/src/traits.rs
+557 −0 fixtures/coverall-upstream-compatibility/tests/bindings/test_coverall_upstream_compatibility.kts
+458 −0 fixtures/coverall-upstream-compatibility/tests/bindings/test_coverall_upstream_compatibility.py
+262 −0 fixtures/coverall-upstream-compatibility/tests/bindings/test_coverall_upstream_compatibility.rb
+481 −0 fixtures/coverall-upstream-compatibility/tests/bindings/test_coverall_upstream_compatibility.swift
+52 −0 fixtures/coverall-upstream-compatibility/tests/bindings/test_handlerace_upstream_compatibility.kts
+9 −0 fixtures/coverall-upstream-compatibility/tests/test_generated_bindings.rs
+3 −1 fixtures/futures/tests/test_generated_bindings.rs
+5 −5 uniffi/Cargo.toml
+0 −8 uniffi/release.toml
+4 −4 uniffi_bindgen/Cargo.toml
+1 −1 uniffi_bindgen/src/bindings/swift/mod.rs
+6 −5 uniffi_bindgen/src/interface/ffi.rs
+10 −10 uniffi_bindgen/src/interface/mod.rs
+1 −1 uniffi_bindgen/src/interface/object.rs
+2 −2 uniffi_build/Cargo.toml
+1 −1 uniffi_checksum_derive/Cargo.toml
+1 −1 uniffi_core/Cargo.toml
+1 −7 uniffi_core/release.toml
+3 −3 uniffi_macros/Cargo.toml
+2 −2 uniffi_meta/Cargo.toml
+1 −1 uniffi_testing/Cargo.toml
+3 −3 uniffi_udl/Cargo.toml
18 changes: 9 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindgen/src/gen_cs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
22 changes: 10 additions & 12 deletions bindgen/templates/ObjectRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<THandle>: IDisposable where THandle : FFISafeHandle {
private THandle handle;
Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 16 additions & 10 deletions bindgen/templates/ObjectTemplate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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) %}
Expand All @@ -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 %}
Expand All @@ -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 %}
Expand All @@ -76,28 +76,34 @@ 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) {
return 8;
}

public override void Write({{ type_name }} value, BigEndianStream stream) {
stream.WriteLong(Lower(value).DangerousGetRawFfiValue().ToInt64());
stream.WriteLong(Lower(value).ToInt64());
}
}
4 changes: 2 additions & 2 deletions bindgen/templates/macros.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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) %}
Expand Down

0 comments on commit b485b2f

Please sign in to comment.