Skip to content

Commit

Permalink
Fixup callbacks deadlock when calling callback very frequently (#158)
Browse files Browse the repository at this point in the history
Fixes #157

Minimal test: 146cd39

The change here is in `UniffiCallInvoker`, reverting the implementation
of `invokeBlocking` back to the Promise based implementation introduced
in PR #88.

The deadlock rate for the test is now 0/50.

/cc @tayrevor

Adding any more to the implementation (e.g. uncommenting the
`console.log`) causes the same error as before, but it is completely
deterministic:

```sh
-- Going to 1024, now at: 264
-- Going to 1024, now at: 265
Assertion failed: (usedDbg_ && "Stats not submitted."), function ~CollectionStats, file HadesGC.cpp, line 259.
```

On my machine it _always_ stops after 265/1024. I am still unsure if
this is a problem with production uniff-bindgen-react-native code, an
artifact of the test-harness or something weirder.

I'm going to suggest that this is addressed in a separate issue.
  • Loading branch information
jhugman authored Nov 12, 2024
1 parent 28f931b commit bd3fb0f
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 13 deletions.
20 changes: 7 additions & 13 deletions cpp/includes/UniffiCallInvoker.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#pragma once
#include <ReactCommon/CallInvoker.h>
#include <condition_variable>
#include <future>
#include <jsi/jsi.h>
#include <memory>
#include <mutex>
Expand Down Expand Up @@ -58,26 +59,19 @@ class UniffiCallInvoker {
if (std::this_thread::get_id() == threadId_) {
func(rt);
} else {
std::mutex mtx;
std::condition_variable cv;
bool done = false;
std::promise<void> promise;
auto future = promise.get_future();
// The runtime argument was added to CallFunc in
// https://github.com/facebook/react-native/pull/43375
//
// This can be changed once that change is released.
// react::CallFunc wrapper = [&func, &mtx, &cv, &done](jsi::Runtime &rt) {
std::function<void()> wrapper = [&func, &rt, &mtx, &cv, &done]() {
// react::CallFunc wrapper = [&func, &promise](jsi::Runtime &rt) {
std::function<void()> wrapper = [&func, &promise, &rt]() {
func(rt);
{
std::lock_guard<std::mutex> lock(mtx);
done = true;
}
cv.notify_one();
promise.set_value();
};
callInvoker_->invokeAsync(std::move(wrapper));

std::unique_lock<std::mutex> lock(mtx);
cv.wait(lock, [&done] { return done; });
future.wait();
}
}

Expand Down
20 changes: 20 additions & 0 deletions fixtures/callbacks-regression/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[package]
name = "uniffi-example-callbacks"
edition = "2021"
version = "0.22.0"
license = "MPL-2.0"
publish = false

[lib]
crate-type = ["lib", "cdylib"]
name = "uniffi_callbacks"

[dependencies]
uniffi = { workspace = true }
thiserror = "1.0"

[build-dependencies]
uniffi = { workspace = true, features = ["build"] }

[dev-dependencies]
uniffi = { workspace = true, features = ["bindgen-tests"] }
5 changes: 5 additions & 0 deletions fixtures/callbacks-regression/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This is an example of UniFFI "callbacks".

Callbacks are the ability to implement Rust traits in foreign languages. These are defined by
[normal UniFFI traits](../../docs/manual/src/foreign_traits.md) or via ["callback" definitions](../../docs/manual/src/udl/callback_interfaces.md).

7 changes: 7 additions & 0 deletions fixtures/callbacks-regression/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/
*/

fn main() {}
35 changes: 35 additions & 0 deletions fixtures/callbacks-regression/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/
*/

use std::{sync::Arc, thread::spawn};

uniffi::setup_scaffolding!();

#[uniffi::export(with_foreign)]
pub trait EventListener: Send + Sync {
fn on_event(&self, message: String, number: i32);
}

#[derive(uniffi::Object)]
struct EventSource {
listener: Arc<dyn EventListener>,
}

#[uniffi::export]
impl EventSource {
#[uniffi::constructor]
fn new(listener: Arc<dyn EventListener>) -> Arc<Self> {
Arc::new(Self { listener })
}
fn start(self: Arc<Self>, message: String, until: i32) {
let listener = self.listener.clone();
spawn(move || {
for i in 0..until {
listener.on_event(message.clone(), i);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/
*/

/*
fixture=fixtures/callbacks-regression
cargo xtask run \
--crate $fixture \
--ts-dir $fixture/generated \
--cpp-dir $fixture/generated/cpp \
$fixture/tests/bindings/test_callbacks_regression.ts
*/

import generated, {
EventSource,
EventListener,
} from "../../generated/uniffi_callbacks";
import { asyncTest, AsyncAsserts } from "@/asserts";
import { console } from "@/hermes";

// This is only needed in tests.
generated.initialize();

class EventListenerImpl implements EventListener {
constructor(
private t: AsyncAsserts,
private max: number,
) {}
onEvent(message: string, number: number): void {
// console.log("--", message, number);
if (number === this.max - 1) {
console.log("-- Done!", this.max);
this.t.end();
}
}
}

async function testToMax(max: number, t: AsyncAsserts) {
const listener = new EventListenerImpl(t, max);
const source = new EventSource(listener);
source.start(`Going to ${max}, now at:`, max);
}

(async () => {
for (let i = 1; i <= 1024; i *= 2) {
await asyncTest(
`Full tilt test up to ${i}`,
async (t) => await testToMax(i, t),
);
}
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/
*/
Empty file.

0 comments on commit bd3fb0f

Please sign in to comment.