diff --git a/Cargo.lock b/Cargo.lock index e61b3a852..93110116f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1368,6 +1368,15 @@ dependencies = [ "serde_core", ] +[[package]] +name = "input_scratch" +version = "0.1.0" +dependencies = [ + "anyhow", + "uniffi", + "uniffi-dart", +] + [[package]] name = "instant" version = "0.1.13" diff --git a/Cargo.toml b/Cargo.toml index 69285006f..755e5956e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ members = [ "uniffi/cpp/fixtures/type-flattening", "uniffi/dart", "uniffi/dart/fixtures/arithmetic", + "uniffi/dart/fixtures/input-scratch", "uniffi/dart/fixtures/benchmarks", "uniffi/dart/fixtures/bytes_types", "uniffi/dart/fixtures/callbacks", diff --git a/uniffi/README.md b/uniffi/README.md index 32291965c..593ef6701 100644 --- a/uniffi/README.md +++ b/uniffi/README.md @@ -65,6 +65,10 @@ Offering a change upstream needs maintainer approval each time, so most have no uniffi-rs: +- Python prefers the running event loop to the configured callback fallback + ([mozilla/uniffi-rs@f903c36c](https://github.com/mozilla/uniffi-rs/commit/f903c36c09fd396560e4dc28763a97a5c552e1c3)); the public setter keeps its 0.32 name. + Unlike upstream, an async callback still goes to the configured loop while it runs, since + a sync call may block the caller's loop on the callback. - Kotlin refuses a destroyed object passed as an argument (kixelated/uniffi-rs@88d835d). - Ruby is removed. - `toml` is pinned to major 1. Upstream's `>=0.9, <2` spans two majors, so a generator that @@ -109,6 +113,10 @@ uniffi-dart, on top of upstream `main` at acterglobal/uniffi-dart@72d26c1: - RustBuffer and async result leak fixes, `liftAndFree` type arguments, non-null optionals lowered through `toRustBuffer`, an exported `Display` as an enum's `toString()`, and disposed objects refused as arguments. +- Input scratch is released on partial allocation failure, and disposal reserves status + storage before closing the handle, so allocation failure leaves it owned and retryable + ([Uniffi-Dart/uniffi-dart@71dc9f8b](https://github.com/Uniffi-Dart/uniffi-dart/commit/71dc9f8b8f54cdfa6ef5d08f24e70cd93ef11d7d), + [@a2751c4c](https://github.com/Uniffi-Dart/uniffi-dart/commit/a2751c4cb0df3d28b5efab0bac1dadf439d9b163)). - Record errors keep their converter name, and futures cancel per operation (moq's `nix/uniffi-dart-record-error.patch` and `nix/uniffi-dart-cancellation.patch`). - Another crate's library is imported with a `uniffi_` prefix that its types, diff --git a/uniffi/dart/fixtures/callbacks/src/lib.rs b/uniffi/dart/fixtures/callbacks/src/lib.rs index 27075aa33..affdbe18b 100644 --- a/uniffi/dart/fixtures/callbacks/src/lib.rs +++ b/uniffi/dart/fixtures/callbacks/src/lib.rs @@ -155,6 +155,20 @@ pub struct InMemoryEventPersister { events: Mutex>, } +static PERSISTER_DROP_COUNT: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + +impl Drop for InMemoryEventPersister { + fn drop(&mut self) { + PERSISTER_DROP_COUNT.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + } +} + +#[uniffi::export] +pub fn persister_drop_count() -> u64 { + PERSISTER_DROP_COUNT.load(std::sync::atomic::Ordering::SeqCst) as u64 +} + #[uniffi::export] impl InMemoryEventPersister { #[uniffi::constructor] diff --git a/uniffi/dart/fixtures/callbacks/test/disposal_allocation_cases.dart b/uniffi/dart/fixtures/callbacks/test/disposal_allocation_cases.dart new file mode 100644 index 000000000..b355cebc9 --- /dev/null +++ b/uniffi/dart/fixtures/callbacks/test/disposal_allocation_cases.dart @@ -0,0 +1,28 @@ +import 'package:test/test.dart'; + +import '../callbacks.dart'; +import 'tracking_allocator.dart' as tracker; + +void main() { + tearDown(tracker.calloc.reset); + + test( + 'foreign trait disposal stays retryable when status allocation fails', + () { + final before = persisterDropCount(); + final owner = InMemoryEventPersister(); + final persister = owner.asPersister(); + tracker.calloc.failAfter = 0; + expect(() => (persister as dynamic).dispose(), throwsStateError); + tracker.calloc.failAfter = null; + (persister as dynamic).dispose(); + expect(() => persister.load(), throwsStateError); + (persister as dynamic).dispose(); + // The owner still holds its own reference. + expect(persisterDropCount(), before); + owner.dispose(); + expect(persisterDropCount(), before + 1); + expect(tracker.calloc.live, isEmpty); + }, + ); +} diff --git a/uniffi/dart/fixtures/callbacks/test/disposal_allocation_test.dart b/uniffi/dart/fixtures/callbacks/test/disposal_allocation_test.dart new file mode 100644 index 000000000..714881d3d --- /dev/null +++ b/uniffi/dart/fixtures/callbacks/test/disposal_allocation_test.dart @@ -0,0 +1,64 @@ +import 'dart:io'; + +import 'package:test/test.dart'; + +void main() { + test('disposal survives injected status allocation failure', () async { + // Run a child suite in an isolated copy of this package with an + // instrumented allocator. The child's build hooks replace the native + // library, so it must not share this run's package directory. + final work = Directory.systemTemp.createTempSync('disposal_allocation'); + try { + await _copyPackage(Directory.current, work); + final pubGet = await Process.run(Platform.resolvedExecutable, [ + 'pub', + 'get', + '--offline', + ], workingDirectory: work.path); + print(pubGet.stdout); + print(pubGet.stderr); + expect(pubGet.exitCode, 0); + for (final name in ['callbacks.dart', 'uniffi_runtime.dart']) { + final file = File('${work.path}/$name'); + final source = file.readAsStringSync(); + final import = RegExp(r'''import ["']package:ffi/ffi.dart["'];'''); + expect(import.allMatches(source), hasLength(1)); + file.writeAsStringSync( + source.replaceFirst( + import, + 'import "package:ffi/ffi.dart" hide calloc;\n' + 'import "test/tracking_allocator.dart";', + ), + ); + } + final result = await Process.run(Platform.resolvedExecutable, [ + 'test', + 'test/disposal_allocation_cases.dart', + '--reporter', + 'expanded', + ], workingDirectory: work.path); + print(result.stdout); + print(result.stderr); + expect(result.exitCode, 0); + } finally { + work.deleteSync(recursive: true); + } + }, timeout: Timeout(Duration(minutes: 3))); +} + +Future _copyPackage(Directory from, Directory to) async { + to.createSync(recursive: true); + for (final entity in from.listSync()) { + final name = entity.uri.pathSegments + .where((segment) => segment.isNotEmpty) + .last; + if (name == '.dart_tool') { + continue; + } + if (entity is Directory) { + await _copyPackage(entity, Directory('${to.path}/$name')); + } else { + File(entity.path).copySync('${to.path}/$name'); + } + } +} diff --git a/uniffi/dart/fixtures/callbacks/test/tracking_allocator.dart b/uniffi/dart/fixtures/callbacks/test/tracking_allocator.dart new file mode 100644 index 000000000..e661da86b --- /dev/null +++ b/uniffi/dart/fixtures/callbacks/test/tracking_allocator.dart @@ -0,0 +1,34 @@ +import 'dart:ffi'; + +import 'package:ffi/ffi.dart' as ffi; + +class TrackingAllocator implements Allocator { + final live = {}; + int? failAfter; + + @override + Pointer allocate(int byteCount, {int? alignment}) { + if (failAfter == 0) throw StateError('injected allocation failure'); + if (failAfter != null) failAfter = failAfter! - 1; + final pointer = ffi.calloc.allocate(byteCount, alignment: alignment); + live[pointer.address] = byteCount; + return pointer; + } + + @override + void free(Pointer pointer) { + if (live.remove(pointer.address) == null) { + throw StateError('free of untracked pointer'); + } + ffi.calloc.free(pointer); + } + + void reset() { + failAfter = null; + for (final address in live.keys.toList()) { + free(Pointer.fromAddress(address)); + } + } +} + +final calloc = TrackingAllocator(); diff --git a/uniffi/dart/fixtures/input-scratch/Cargo.toml b/uniffi/dart/fixtures/input-scratch/Cargo.toml new file mode 100644 index 000000000..996e72c6b --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "input_scratch" +version = "0.1.0" +edition = "2021" +publish = false +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + + +[lib] +name = "input_scratch" +crate-type = ["lib", "cdylib"] + +[dependencies] +uniffi = { workspace = true } + +[build-dependencies] +uniffi-dart = { path = "../../", features = ["build"] } + +[dev-dependencies] +uniffi-dart = { path = "../../", features = ["bindgen-tests"] } +uniffi = { workspace = true, features = ["bindgen-tests"] } +anyhow = "1" diff --git a/uniffi/dart/fixtures/input-scratch/build.rs b/uniffi/dart/fixtures/input-scratch/build.rs new file mode 100644 index 000000000..17e394981 --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/build.rs @@ -0,0 +1,3 @@ +fn main() { + uniffi_dart::generate_scaffolding("./src/api.udl".into()).unwrap(); +} diff --git a/uniffi/dart/fixtures/input-scratch/src/api.udl b/uniffi/dart/fixtures/input-scratch/src/api.udl new file mode 100644 index 000000000..430909683 --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/src/api.udl @@ -0,0 +1 @@ +namespace input_scratch {}; diff --git a/uniffi/dart/fixtures/input-scratch/src/lib.rs b/uniffi/dart/fixtures/input-scratch/src/lib.rs new file mode 100644 index 000000000..7736d82da --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/src/lib.rs @@ -0,0 +1,16 @@ +#[uniffi::export] +pub fn owned_len(value: Vec) -> u64 { + value.len() as u64 +} + +#[uniffi::export] +pub fn optional_len(value: Option) -> u64 { + value.map_or(0, |s| s.len() as u64) +} + +#[uniffi::export] +pub fn optional_small(value: Option) -> u8 { + value.unwrap_or(0) +} + +uniffi::include_scaffolding!("api"); diff --git a/uniffi/dart/fixtures/input-scratch/test/input_scratch_test.dart b/uniffi/dart/fixtures/input-scratch/test/input_scratch_test.dart new file mode 100644 index 000000000..d6a138172 --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/test/input_scratch_test.dart @@ -0,0 +1,40 @@ +import 'dart:io'; + +import 'package:test/test.dart'; + +void main() { + test('scratch allocations balance on success and injected failure', () async { + // Compile a child suite with an instrumented allocator. Keep the production + // API unchanged and restore the generated files even when a test fails. + final originals = {}; + try { + for (final name in ['input_scratch.dart', 'uniffi_runtime.dart']) { + final file = File(name); + final source = file.readAsStringSync(); + originals[file] = source; + final import = RegExp(r'''import ["']package:ffi/ffi.dart["'];'''); + expect(import.allMatches(source), hasLength(1)); + file.writeAsStringSync( + source.replaceFirst( + import, + 'import "package:ffi/ffi.dart" hide calloc;\n' + 'import "test/tracking_allocator.dart";', + ), + ); + } + final result = await Process.run(Platform.resolvedExecutable, [ + 'test', + 'test/scratch_cases.dart', + '--reporter', + 'expanded', + ]); + print(result.stdout); + print(result.stderr); + expect(result.exitCode, 0); + } finally { + for (final entry in originals.entries) { + entry.key.writeAsStringSync(entry.value); + } + } + }, timeout: Timeout(Duration(minutes: 3))); +} diff --git a/uniffi/dart/fixtures/input-scratch/test/scratch_cases.dart b/uniffi/dart/fixtures/input-scratch/test/scratch_cases.dart new file mode 100644 index 000000000..728210290 --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/test/scratch_cases.dart @@ -0,0 +1,66 @@ +import 'dart:typed_data'; + +import 'package:test/test.dart'; + +import '../input_scratch.dart'; +import 'tracking_allocator.dart' as tracker; + +void main() { + ensureInitialized(); + tearDown(tracker.calloc.reset); + + test( + 'owned and optional inputs release scratch after native consumption', + () { + for (var i = 0; i < 100; i++) { + expect(ownedLen(value: Uint8List.fromList([1, 2, 3])), 3); + expect(ownedLen(value: Uint8List(0)), 0); + expect(optionalLen(value: 'x' * 64), 64); + expect(optionalLen(value: ''), 0); + expect(optionalLen(value: null), 0); + expect(tracker.calloc.live, isEmpty); + } + }, + ); + + test('scratch cleanup preserves the copied RustBuffer', () { + final input = Uint8List.fromList([1, 2, 3]); + final buffer = toRustBuffer(input); + try { + expect(tracker.calloc.live, isEmpty); + input[0] = 99; + expect(buffer.asUint8List(), [1, 2, 3]); + } finally { + buffer.free(); + } + expect(tracker.calloc.live, isEmpty); + }); + + for (final optional in [false, true]) { + for (var failAt = 0; failAt < 3; failAt++) { + test( + 'partial allocation failure releases scratch: optional=$optional, allocation=$failAt', + () { + // Payload, ForeignBytes, and RustCallStatus are allocated in that order. + tracker.calloc.failAfter = failAt; + try { + expect( + () => optional + ? FfiConverterOptionalString.lower('x' * 64) + : toRustBuffer(Uint8List(64)), + throwsStateError, + ); + } finally { + tracker.calloc.failAfter = null; + } + expect(tracker.calloc.live, isEmpty); + }, + ); + } + } + + test('optional serialization failure retains no native scratch', () { + expect(() => optionalSmall(value: 256), throwsArgumentError); + expect(tracker.calloc.live, isEmpty); + }); +} diff --git a/uniffi/dart/fixtures/input-scratch/test/tracking_allocator.dart b/uniffi/dart/fixtures/input-scratch/test/tracking_allocator.dart new file mode 100644 index 000000000..e661da86b --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/test/tracking_allocator.dart @@ -0,0 +1,34 @@ +import 'dart:ffi'; + +import 'package:ffi/ffi.dart' as ffi; + +class TrackingAllocator implements Allocator { + final live = {}; + int? failAfter; + + @override + Pointer allocate(int byteCount, {int? alignment}) { + if (failAfter == 0) throw StateError('injected allocation failure'); + if (failAfter != null) failAfter = failAfter! - 1; + final pointer = ffi.calloc.allocate(byteCount, alignment: alignment); + live[pointer.address] = byteCount; + return pointer; + } + + @override + void free(Pointer pointer) { + if (live.remove(pointer.address) == null) { + throw StateError('free of untracked pointer'); + } + ffi.calloc.free(pointer); + } + + void reset() { + failAfter = null; + for (final address in live.keys.toList()) { + free(Pointer.fromAddress(address)); + } + } +} + +final calloc = TrackingAllocator(); diff --git a/uniffi/dart/fixtures/input-scratch/tests/mod.rs b/uniffi/dart/fixtures/input-scratch/tests/mod.rs new file mode 100644 index 000000000..a3c33dbb9 --- /dev/null +++ b/uniffi/dart/fixtures/input-scratch/tests/mod.rs @@ -0,0 +1,4 @@ +#[test] +fn input_scratch() -> anyhow::Result<()> { + uniffi_dart::testing::run_test("input_scratch", "src/api.udl", None) +} diff --git a/uniffi/dart/fixtures/trait-interfaces/src/lib.rs b/uniffi/dart/fixtures/trait-interfaces/src/lib.rs index b6eb4d492..65cb3f0ea 100644 --- a/uniffi/dart/fixtures/trait-interfaces/src/lib.rs +++ b/uniffi/dart/fixtures/trait-interfaces/src/lib.rs @@ -93,4 +93,43 @@ impl Registry { } } +// Keep lifecycle counts separate from the other fixture objects/finalizers. +static DISPOSAL_DROPS: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + +#[derive(uniffi::Object)] +pub struct DisposalProbe; + +#[uniffi::export] +impl DisposalProbe { + #[uniffi::constructor] + pub fn new() -> Arc { + Arc::new(Self) + } + + pub fn greet(&self, name: String) -> String { + format!("Hello {name}") + } + + pub fn as_greeter(self: Arc) -> Arc { + self + } +} + +impl Greeter for DisposalProbe { + fn greet(&self, name: String) -> String { + self.greet(name) + } +} + +impl Drop for DisposalProbe { + fn drop(&mut self) { + DISPOSAL_DROPS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + } +} + +#[uniffi::export] +pub fn disposal_drop_count() -> u32 { + DISPOSAL_DROPS.load(std::sync::atomic::Ordering::SeqCst) +} + uniffi::include_scaffolding!("api"); diff --git a/uniffi/dart/fixtures/trait-interfaces/test/disposal_allocation_cases.dart b/uniffi/dart/fixtures/trait-interfaces/test/disposal_allocation_cases.dart new file mode 100644 index 000000000..00ca39c6e --- /dev/null +++ b/uniffi/dart/fixtures/trait-interfaces/test/disposal_allocation_cases.dart @@ -0,0 +1,37 @@ +import 'package:test/test.dart'; + +import '../trait_interfaces.dart'; +import 'tracking_allocator.dart' as tracker; + +void main() { + tearDown(tracker.calloc.reset); + + test('object disposal stays retryable when status allocation fails', () { + final before = disposalDropCount(); + final probe = DisposalProbe(); + tracker.calloc.failAfter = 0; + expect(() => probe.dispose(), throwsStateError); + tracker.calloc.failAfter = null; + probe.dispose(); + expect(disposalDropCount(), before + 1); + probe.dispose(); + expect(disposalDropCount(), before + 1); + expect(tracker.calloc.live, isEmpty); + }); + + test('trait disposal stays retryable when status allocation fails', () { + final before = disposalDropCount(); + final probe = DisposalProbe(); + final greeter = probe.asGreeter(); + tracker.calloc.failAfter = 0; + expect(() => greeter.dispose(), throwsStateError); + tracker.calloc.failAfter = null; + greeter.dispose(); + greeter.dispose(); + // The probe still holds its own reference. + expect(disposalDropCount(), before); + probe.dispose(); + expect(disposalDropCount(), before + 1); + expect(tracker.calloc.live, isEmpty); + }); +} diff --git a/uniffi/dart/fixtures/trait-interfaces/test/disposal_allocation_test.dart b/uniffi/dart/fixtures/trait-interfaces/test/disposal_allocation_test.dart new file mode 100644 index 000000000..28e65101b --- /dev/null +++ b/uniffi/dart/fixtures/trait-interfaces/test/disposal_allocation_test.dart @@ -0,0 +1,64 @@ +import 'dart:io'; + +import 'package:test/test.dart'; + +void main() { + test('disposal survives injected status allocation failure', () async { + // Run a child suite in an isolated copy of this package with an + // instrumented allocator. The child's build hooks replace the native + // library, so it must not share this run's package directory. + final work = Directory.systemTemp.createTempSync('disposal_allocation'); + try { + await _copyPackage(Directory.current, work); + final pubGet = await Process.run(Platform.resolvedExecutable, [ + 'pub', + 'get', + '--offline', + ], workingDirectory: work.path); + print(pubGet.stdout); + print(pubGet.stderr); + expect(pubGet.exitCode, 0); + for (final name in ['trait_interfaces.dart', 'uniffi_runtime.dart']) { + final file = File('${work.path}/$name'); + final source = file.readAsStringSync(); + final import = RegExp(r'''import ["']package:ffi/ffi.dart["'];'''); + expect(import.allMatches(source), hasLength(1)); + file.writeAsStringSync( + source.replaceFirst( + import, + 'import "package:ffi/ffi.dart" hide calloc;\n' + 'import "test/tracking_allocator.dart";', + ), + ); + } + final result = await Process.run(Platform.resolvedExecutable, [ + 'test', + 'test/disposal_allocation_cases.dart', + '--reporter', + 'expanded', + ], workingDirectory: work.path); + print(result.stdout); + print(result.stderr); + expect(result.exitCode, 0); + } finally { + work.deleteSync(recursive: true); + } + }, timeout: Timeout(Duration(minutes: 3))); +} + +Future _copyPackage(Directory from, Directory to) async { + to.createSync(recursive: true); + for (final entity in from.listSync()) { + final name = entity.uri.pathSegments + .where((segment) => segment.isNotEmpty) + .last; + if (name == '.dart_tool') { + continue; + } + if (entity is Directory) { + await _copyPackage(entity, Directory('${to.path}/$name')); + } else { + File(entity.path).copySync('${to.path}/$name'); + } + } +} diff --git a/uniffi/dart/fixtures/trait-interfaces/test/tracking_allocator.dart b/uniffi/dart/fixtures/trait-interfaces/test/tracking_allocator.dart new file mode 100644 index 000000000..e661da86b --- /dev/null +++ b/uniffi/dart/fixtures/trait-interfaces/test/tracking_allocator.dart @@ -0,0 +1,34 @@ +import 'dart:ffi'; + +import 'package:ffi/ffi.dart' as ffi; + +class TrackingAllocator implements Allocator { + final live = {}; + int? failAfter; + + @override + Pointer allocate(int byteCount, {int? alignment}) { + if (failAfter == 0) throw StateError('injected allocation failure'); + if (failAfter != null) failAfter = failAfter! - 1; + final pointer = ffi.calloc.allocate(byteCount, alignment: alignment); + live[pointer.address] = byteCount; + return pointer; + } + + @override + void free(Pointer pointer) { + if (live.remove(pointer.address) == null) { + throw StateError('free of untracked pointer'); + } + ffi.calloc.free(pointer); + } + + void reset() { + failAfter = null; + for (final address in live.keys.toList()) { + free(Pointer.fromAddress(address)); + } + } +} + +final calloc = TrackingAllocator(); diff --git a/uniffi/dart/src/gen/objects.rs b/uniffi/dart/src/gen/objects.rs index 82da4906c..8704d0c3c 100644 --- a/uniffi/dart/src/gen/objects.rs +++ b/uniffi/dart/src/gen/objects.rs @@ -310,9 +310,17 @@ pub fn generate_object(obj: &Object, type_helper: &dyn TypeHelperRenderer) -> da if (_isDisposed) { return; } + // Reserve status storage before closing, so allocation failure + // leaves the handle owned and disposal retryable. + final status = calloc(); _isDisposed = true; _$finalizer_cls_name.detach(this); - rustCall((status) => $ffi_object_free_name(_ptr, status)); + try { + $ffi_object_free_name(_ptr, status); + checkCallStatus(NullRustCallStatusErrorHandler(), status); + } finally { + calloc.free(status); + } } $to_string_method @@ -393,9 +401,17 @@ fn generate_callback_trait_rust_impl( if (_isDisposed) { return; } + // Reserve status storage before closing, so allocation failure + // leaves the handle owned and disposal retryable. + final status = calloc(); _isDisposed = true; $(&finalizer_field).detach(this); - rustCall((status) => $ffi_object_free_name(_ptr, status)); + try { + $ffi_object_free_name(_ptr, status); + checkCallStatus(NullRustCallStatusErrorHandler(), status); + } finally { + calloc.free(status); + } } $(for method in concrete_methods => $method) @@ -814,9 +830,17 @@ fn generate_trait_object(obj: &Object, type_helper: &dyn TypeHelperRenderer) -> if (_isDisposed) { return; } + // Reserve status storage before closing, so allocation failure + // leaves the handle owned and disposal retryable. + final status = calloc(); _isDisposed = true; $(&finalizer_field).detach(this); - rustCall((status) => $ffi_object_free_name(_ptr, status)); + try { + $ffi_object_free_name(_ptr, status); + checkCallStatus(NullRustCallStatusErrorHandler(), status); + } finally { + calloc.free(status); + } } $(for method in concrete_methods => $method) diff --git a/uniffi/dart/src/gen/types.rs b/uniffi/dart/src/gen/types.rs index 4ee8366a6..0319b07b1 100644 --- a/uniffi/dart/src/gen/types.rs +++ b/uniffi/dart/src/gen/types.rs @@ -429,20 +429,19 @@ pub fn runtime_scaffolding(ci: &ComponentInterface) -> dart::Tokens { RustBuffer toRustBuffer(Uint8List data) { final length = data.length; - final Pointer frameData = calloc(length); // Allocate a pointer large enough. - final pointerList = frameData.asTypedList(length); // Create a list that uses our pointer and copy in the data. - pointerList.setAll(0, data); // FIXME: can we remove this memcopy somehow? - - final bytes = calloc(); - bytes.ref.len = length; - bytes.ref.data = frameData; + final frameData = calloc(length); try { - // Rust copies out of the ForeignBytes, so the scratch above is - // ours to release once fromBytes returns. - return RustBuffer.fromBytes(bytes.ref); + frameData.asTypedList(length).setAll(0, data); + final bytes = calloc(); + try { + bytes.ref.len = length; + bytes.ref.data = frameData; + return RustBuffer.fromBytes(bytes.ref); + } finally { + calloc.free(bytes); + } } finally { calloc.free(frameData); - calloc.free(bytes); } } diff --git a/uniffi/fixtures/futures/src/lib.rs b/uniffi/fixtures/futures/src/lib.rs index 26135d005..062e79026 100644 --- a/uniffi/fixtures/futures/src/lib.rs +++ b/uniffi/fixtures/futures/src/lib.rs @@ -497,6 +497,12 @@ async fn as_string_using_trait(obj: Arc, delay_ms: i32, value: obj.as_string(delay_ms, value).await } +// Blocks the calling thread on an async callback, as a sync callback using `pollster` would. +#[uniffi::export] +fn as_string_using_trait_blocking(obj: Arc, delay_ms: i32, value: i32) -> String { + futures::executor::block_on(obj.as_string(delay_ms, value)) +} + #[uniffi::export] async fn try_from_string_using_trait( obj: Arc, diff --git a/uniffi/fixtures/futures/tests/bindings/test_futures.py b/uniffi/fixtures/futures/tests/bindings/test_futures.py index 7cb102d5e..18f63c073 100644 --- a/uniffi/fixtures/futures/tests/bindings/test_futures.py +++ b/uniffi/fixtures/futures/tests/bindings/test_futures.py @@ -2,6 +2,7 @@ import unittest from datetime import datetime import asyncio +import threading import futures def now(): @@ -291,5 +292,73 @@ async def test(): await use_shared_resource(SharedResourceOptions(release_after_ms=0, timeout_ms=1000)) asyncio.run(test()) + def test_call_from_a_second_event_loop(self): + """A call is awaited on the loop making it, not on whichever one was set first. + + `uniffi_set_event_loop` exists so a thread with no loop of its own has one to schedule a + callback on. A thread that IS running a loop needs no such fallback, and taking it there + builds the poll future on one loop and awaits it from another, which asyncio refuses with + "got Future attached to a different loop". + """ + first = asyncio.new_event_loop() + try: + futures.uniffi_set_event_loop(first) + + async def test(): + self.assertEqual(await always_ready(), True) + + # A second loop, of the kind `asyncio.run` makes: this is the one awaiting the call. + asyncio.run(test()) + finally: + futures.uniffi_set_event_loop(None) + first.close() + + def test_callback_while_the_configured_loop_is_idle(self): + """A callback made on a running loop runs there while the configured loop is not running. + + Nothing would run it on the idle configured loop, so the call awaiting it would never finish. + """ + idle = asyncio.new_event_loop() + try: + futures.uniffi_set_event_loop(idle) + + async def test(): + self.assertEqual(await as_string_using_trait(EchoParser(), 1, 42), "42") + + asyncio.run(test()) + finally: + futures.uniffi_set_event_loop(None) + idle.close() + + def test_blocking_callback_from_a_running_loop(self): + """A callback that blocks this thread's loop runs on the configured loop. + + The sync call holds the running loop until the callback finishes, so the callback cannot run + there. + """ + configured = asyncio.new_event_loop() + running = threading.Event() + configured.call_soon(running.set) + thread = threading.Thread(target=configured.run_forever) + thread.start() + try: + running.wait() + futures.uniffi_set_event_loop(configured) + + async def test(): + self.assertEqual(as_string_using_trait_blocking(EchoParser(), 1, 42), "42") + + asyncio.run(test()) + finally: + futures.uniffi_set_event_loop(None) + configured.call_soon_threadsafe(configured.stop) + thread.join() + configured.close() + +class EchoParser(AsyncParser): + async def as_string(self, delay_ms, value): + await asyncio.sleep(delay_ms / 1000.0) + return str(value) + if __name__ == '__main__': unittest.main() diff --git a/uniffi/uniffi_bindgen/templates/python/Async.py b/uniffi/uniffi_bindgen/templates/python/Async.py index 4ac58776b..482d16dfb 100644 --- a/uniffi/uniffi_bindgen/templates/python/Async.py +++ b/uniffi/uniffi_bindgen/templates/python/Async.py @@ -18,16 +18,29 @@ In this case, we need an event loop to run the Python async function, but there's no eventloop set for the thread. Use `uniffi_set_event_loop` to force an eventloop to be used in this case. +While that loop runs, async callbacks are scheduled on it even from a thread running its own loop, +since a sync call may block that loop on the callback. """ def uniffi_set_event_loop(eventloop: asyncio.BaseEventLoop): global _UNIFFI_GLOBAL_EVENT_LOOP _UNIFFI_GLOBAL_EVENT_LOOP = eventloop def _uniffi_get_event_loop(): - if _UNIFFI_GLOBAL_EVENT_LOOP is not None: - return _UNIFFI_GLOBAL_EVENT_LOOP - else: + # Futures belong to the loop running this call. The configured loop is only + # a fallback for threads without their own event loop. + try: return asyncio.get_running_loop() + except RuntimeError: + if _UNIFFI_GLOBAL_EVENT_LOOP is not None: + return _UNIFFI_GLOBAL_EVENT_LOOP + raise + +def _uniffi_get_callback_event_loop(): + # A sync Rust call may block this thread's loop until the callback finishes, so a running + # configured loop takes the callback. One that is not running would never run it. + if _UNIFFI_GLOBAL_EVENT_LOOP is not None and _UNIFFI_GLOBAL_EVENT_LOOP.is_running(): + return _UNIFFI_GLOBAL_EVENT_LOOP + return _uniffi_get_event_loop() # Continuation callback for async functions # lift the return value or error and resolve the future, causing the async function to resume. @@ -84,7 +97,7 @@ async def make_call_and_call_callback(): ) else: handle_success(call_result) - eventloop = _uniffi_get_event_loop() + eventloop = _uniffi_get_callback_event_loop() task = asyncio.run_coroutine_threadsafe(make_call_and_call_callback(), eventloop) handle = _UNIFFI_FOREIGN_FUTURE_HANDLE_MAP.insert((eventloop, task)) uniffi_out_dropped_callback[0] = _UniffiForeignFutureDroppedCallbackStruct(handle, _uniffi_future_dropped_callback) @@ -110,7 +123,7 @@ async def make_call_and_call_callback(): _UniffiRustCallStatus.CALL_UNEXPECTED_ERROR, {{ builtin_types.string.ffi_converter_name }}.lower(repr(e)), ) - eventloop = _uniffi_get_event_loop() + eventloop = _uniffi_get_callback_event_loop() task = asyncio.run_coroutine_threadsafe(make_call_and_call_callback(), eventloop) handle = _UNIFFI_FOREIGN_FUTURE_HANDLE_MAP.insert((eventloop, task)) uniffi_out_dropped_callback[0] = _UniffiForeignFutureDroppedCallbackStruct(handle, _uniffi_future_dropped_callback)