diff --git a/cpp/includes/UniffiCallInvoker.h b/cpp/includes/UniffiCallInvoker.h index 598305c5..8e27c2ba 100644 --- a/cpp/includes/UniffiCallInvoker.h +++ b/cpp/includes/UniffiCallInvoker.h @@ -5,9 +5,10 @@ */ #pragma once #include -#include +#include #include #include +#include #include namespace uniffi_runtime { @@ -57,23 +58,37 @@ class UniffiCallInvoker { if (std::this_thread::get_id() == threadId_) { func(rt); } else { - std::promise promise; - auto future = promise.get_future(); + std::mutex mtx; + std::condition_variable cv; + bool done = false; // The runtime argument was added to CallFunc in // https://github.com/facebook/react-native/pull/43375 // - // Once that is released, there will be a deprecation period. - // - // Any time during the deprecation period, we can switch `&rt` - // from being a captured variable to being an argument, i.e. - // commenting out one line, and uncommenting the other. - std::function wrapper = [&func, &promise, &rt]() { - // react::CallFunc wrapper = [&func, &promise](jsi::Runtime &rt) { + // This can be changed once that change is released. + // react::CallFunc wrapper = [&func, &mtx, &cv, &done](jsi::Runtime &rt) { + std::function wrapper = [&func, &rt, &mtx, &cv, &done]() { + func(rt); + { + std::lock_guard lock(mtx); + done = true; + } + cv.notify_one(); + }; + callInvoker_->invokeAsync(std::move(wrapper)); + + std::unique_lock lock(mtx); + cv.wait(lock, [&done] { return done; }); + } + } + + void invokeAsync(jsi::Runtime &rt, UniffiCallFunc &func) { + if (std::this_thread::get_id() == threadId_) { + func(rt); + } else { + std::function wrapper = [&func, &rt]() { func(rt); - promise.set_value(); }; callInvoker_->invokeAsync(std::move(wrapper)); - future.wait(); } } }; diff --git a/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/CallbackFunction.cpp b/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/CallbackFunction.cpp index 20aadbd2..156ff367 100644 --- a/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/CallbackFunction.cpp +++ b/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/CallbackFunction.cpp @@ -148,10 +148,11 @@ namespace {{ ns }} { makeCallbackFunction( // {{ ns }} jsi::Runtime &rt, std::shared_ptr callInvoker, - const jsi::Value &value) { + const jsi::Value &value, + bool invokeAsync = false) { auto callbackFunction = value.asObject(rt).asFunction(rt); auto callbackValue = std::make_shared(rt, callbackFunction); - rsLambda = [&rt, callInvoker, callbackValue]( + rsLambda = [&rt, callInvoker, callbackValue, invokeAsync]( {%- for arg in callback.arguments() %} {%- let arg_t = arg.type_().borrow()|ffi_type_name %} {%- let arg_nm_rs = arg.name()|var_name|fmt("rs_{}") %} @@ -187,7 +188,11 @@ namespace {{ ns }} { }; // We'll then call that lambda from the callInvoker which will // look after calling it on the correct thread. - callInvoker->invokeBlocking(rt, jsLambda); + if (invokeAsync) { + callInvoker->invokeAsync(rt, jsLambda); + } else { + callInvoker->invokeBlocking(rt, jsLambda); + } }; return callback; } diff --git a/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/Future.cpp b/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/Future.cpp index a7de0135..5980f643 100644 --- a/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/Future.cpp +++ b/crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/Future.cpp @@ -14,7 +14,8 @@ template <> struct Bridging { static auto callback = {{ cb_type.borrow()|cpp_namespace(ci) }}::makeCallbackFunction( rt, callInvoker, - value + value, + true ); return callback; } catch (const std::logic_error &e) { diff --git a/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/ErrorTemplate.ts b/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/ErrorTemplate.ts index 25ff5819..ba79e2c6 100644 --- a/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/ErrorTemplate.ts +++ b/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/ErrorTemplate.ts @@ -9,8 +9,16 @@ export const {{ decl_type_name }} = (() => { {%- call ts::docstring(variant, 4) %} {%- let variant_name = variant.name()|class_name(ci) %} class {{ variant_name }} extends UniffiError { - private readonly __uniffiTypeName = "{{ type_name }}"; - private readonly __variant = {{ loop.index }}; + /** + * @private + * This field is private and should not be used. + */ + readonly __uniffiTypeName = "{{ type_name }}"; + /** + * @private + * This field is private and should not be used. + */ + readonly __variant = {{ loop.index }}; constructor(message: string) { super("{{ type_name }}", "{{ variant_name }}", message); } diff --git a/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts b/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts index 3bda8580..789a5400 100644 --- a/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts +++ b/crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/TaggedEnumTemplate.ts @@ -56,7 +56,11 @@ export const {{ decl_type_name }} = (() => { {% call ts::docstring(variant, 4) %} class {{ variant_name }} extends {{ superclass }} implements {{ variant_interface }} { - private readonly __uniffiTypeName = "{{ type_name }}"; + /** + * @private + * This field is private and should not be used, use `tag` instead. + */ + readonly __uniffiTypeName = "{{ type_name }}"; readonly tag = {{ variant_tag }}; {%- if has_fields %} readonly inner: {% call variant_data_type(variant) %}; diff --git a/crates/ubrn_cli/src/config/mod.rs b/crates/ubrn_cli/src/config/mod.rs index 9e5806f6..5172ec72 100644 --- a/crates/ubrn_cli/src/config/mod.rs +++ b/crates/ubrn_cli/src/config/mod.rs @@ -56,11 +56,7 @@ impl ProjectConfig { } fn trim_react_native(name: &str) -> String { - name.strip_prefix("RN") - .unwrap_or(name) - .replace("ReactNative", "") - .replace("react-native", "") - .trim_matches('-') + name.trim_matches('-') .trim_matches('_') .to_string() } @@ -181,7 +177,7 @@ impl TurboModulesConfig { fn default_spec_name() -> String { let package_json = workspace::package_json(); let codegen_name = &package_json.codegen().name; - format!("Native{}", trim_react_native(codegen_name)) + trim_react_native(codegen_name) } } diff --git a/typescript/src/async-rust-call.ts b/typescript/src/async-rust-call.ts index df0e0e21..8d284a0b 100644 --- a/typescript/src/async-rust-call.ts +++ b/typescript/src/async-rust-call.ts @@ -33,6 +33,12 @@ type PollFunc = ( handle: UniffiHandle, ) => void; +// Calls setTimeout and then resolves the promise. +// This may be used as a simple yield. +export async function delayPromise(delayMs: number): Promise { + return new Promise((resolve) => setTimeout(resolve, delayMs)); +} + /** * This method calls an asynchronous method on the Rust side. * @@ -71,7 +77,7 @@ export async function uniffiRustCallAsync( pollResult = await pollRust((handle) => { pollFunc(rustFuture, uniffiFutureContinuationCallback, handle); }); - } while (pollResult != UNIFFI_RUST_FUTURE_POLL_READY); + } while (pollResult !== UNIFFI_RUST_FUTURE_POLL_READY); // Now it's ready, all we need to do is pick up the result (and error). return liftFunc( @@ -85,7 +91,7 @@ export async function uniffiRustCallAsync( // #RUST_TASK_CANCELLATION: the unused `cancelFunc` function should be exposed // to client code in order for clients to be able to cancel the running Rust task. } finally { - freeFunc(rustFuture); + setTimeout(() => freeFunc(rustFuture), 0); } } @@ -110,7 +116,24 @@ const uniffiFutureContinuationCallback: UniffiRustFutureContinuationCallback = ( pollResult: number, ) => { const resolve = UNIFFI_RUST_FUTURE_RESOLVER_MAP.remove(handle); - resolve(pollResult); + if (pollResult === UNIFFI_RUST_FUTURE_POLL_READY) { + resolve(pollResult); + } else { + // From https://github.com/mozilla/uniffi-rs/pull/1837/files#diff-8a28c9cf1245b4f714d406ea4044d68e1000099928eaca1afb504ccbc008fe9fR35-R37 + // + // > WARNING: the call to [rust_future_poll] must be scheduled to happen soon after the callback is + // > called, but not inside the callback itself. If [rust_future_poll] is called inside the + // > callback, some futures will deadlock and our scheduler code might as well. + // + // This delay is to ensure that `uniffiFutureContinuationCallback` returns before the next poll, i.e. + // so that the next poll is outside of this callback. + // + // The length of the delay seems to be significant (at least in tests which hammer a network). + // I would like to understand this more: I am still seeing deadlocks when this drops below its current + // delay, but these maybe related to a different issue, as alluded to in + // https://github.com/mozilla/uniffi-rs/pull/1901 + setTimeout(() => resolve(pollResult), 20); + } }; // For testing only. diff --git a/typescript/tests/playground/enums.ts b/typescript/tests/playground/enums.ts index 38c068d9..9092d1c3 100644 --- a/typescript/tests/playground/enums.ts +++ b/typescript/tests/playground/enums.ts @@ -12,7 +12,7 @@ export const MyEnum = (() => { inner: Readonly<{ myValue: string }>; }; class Variant1_ extends UniffiEnum implements Variant1__interface { - private readonly __uniffiTypeName = typeName; + readonly __uniffiTypeName = typeName; readonly tag = "Variant1"; readonly inner: Readonly<{ myValue: string }>; constructor(inner: { myValue: string }) { @@ -29,7 +29,7 @@ export const MyEnum = (() => { inner: Readonly<[number, string]>; }; class Variant2_ extends UniffiEnum implements Variant2__interface { - private readonly __uniffiTypeName = typeName; + readonly __uniffiTypeName = typeName; readonly tag = "Variant2"; readonly inner: Readonly<[number, string]>; constructor(p1: number, p2: string) {