Skip to content

Commit

Permalink
Fixup deadlock when hammering futures of a certain type
Browse files Browse the repository at this point in the history
  • Loading branch information
jhugman committed Sep 5, 2024
1 parent efe2147 commit f657971
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 2 deletions.
28 changes: 28 additions & 0 deletions fixtures/async-deadlock/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
[package]
name = "regression-async-deadlock"
edition = "2021"
version = "0.0.1"
authors = ["Filament Team <[email protected]>"]
license = "MPL-2.0"
publish = false

[lib]
crate-type = ["lib", "staticlib", "cdylib"]
name = "regression_async_deadlock"

[patch.crates-io]
async-compat = { git = "https://github.com/jplatte/async-compat", rev = "16dc8597ec09a6102d58d4e7b67714a35dd0ecb8" }
const_panic = { git = "https://github.com/jplatte/const_panic", rev = "9024a4cb3eac45c1d2d980f17aaee287b17be498" }

[dependencies]
uniffi = { workspace = true }
matrix-sdk-ffi = { workspace = true }
matrix-sdk = { workspace = true }
thiserror = "1.0"


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

[dev-dependencies]
uniffi = { workspace = true, features = ["bindgen-tests"] }
16 changes: 16 additions & 0 deletions fixtures/async-deadlock/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* 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;

use matrix_sdk_ffi::client_builder::ClientBuilder;

#[uniffi::export]
pub fn get_matrix_client_builder() -> Arc<ClientBuilder> {
ClientBuilder::new()
}

uniffi::setup_scaffolding!();
107 changes: 107 additions & 0 deletions fixtures/async-deadlock/tests/bindings/test_async_deadlock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* 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/
*/

import { asyncTest } from "@/asserts";
import { console } from "@/hermes";
import {
ClientBuilder,
ClientInterface,
mediaSourceFromUrl,
MediaSourceInterface,
} from "../../generated/matrix_sdk_ffi";

import { delayPromise } from "uniffi-bindgen-react-native";

const url = "mxc://matrix.1badday.com/RUUIKNjovSSwYbULWuNQarDA";

(async () => {
await asyncTest("Test delay promise", async (t) => {
console.info("Starting delay");
await delayPromise(0);
const start = Date.now();
await delayPromise(1000);
const actual = Date.now() - start;
console.info(`Ending delay, measured: ${actual} ms`);
t.assertInRange(actual, 900, 1100);

t.end();
});

await asyncTest("Test for deadlock 8", async (t) => {
await loadImages(8);
t.end();
});

await asyncTest("Test for deadlock 16", async (t) => {
await loadImages(16);
t.end();
});


await asyncTest("Test for deadlock 32", async (t) => {
await loadImages(32);
t.end();
});

await asyncTest("Test for deadlock 64", async (t) => {
await loadImages(64);
t.end();
});

await asyncTest("Test for deadlock 128", async (t) => {
await loadImages(128);
t.end();
});

await asyncTest("Test for deadlock 256", async (t) => {
await loadImages(256);
t.end();
});

await asyncTest("Test for deadlock 512", async (t) => {
await loadImages(512);
t.end();
}, 30 * 1000);

await asyncTest("Test for deadlock 1024", async (t) => {
await loadImages(1024);
t.end();
}, 60 * 1000);

})();

async function loadImages(n: number): Promise<void> {
const images = new Array(n).fill(url);
const sourcedImages = images.map((i) => ({
source: mediaSourceFromUrl(i),
}));
const client = await new ClientBuilder()
.homeserverUrl("https://matrix.1badday.com/")
.build();
console.log("Starting…");
const promises = sourcedImages.map((s, i) => {
console.log(`-- About to load image ${i}/${n}: ${s.source.url()}`);
return client.getMediaContent(s.source).then((content) => {
console.log(`-- Loaded image ${i}/${n}`, s.source.url());
});
});
await Promise.allSettled(promises);
console.log("… finished");
}

// (async function () {
// const client = await new ClientBuilder()
// .homeserverUrl("https://matrix.1badday.com/")
// .build();
// const promises = sourcedImages.map((s) => {
// console.log(`-- About to load ${s.source.url}`);
// client.getMediaContent(s.source).then((content) => {
// console.log("-- Loaded image: ", s.source.url());
// });
// });

// await Promise.allSettled(promises);
// })();
3 changes: 3 additions & 0 deletions fixtures/async-deadlock/uniffi.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[bindings.typescript]
isVerbose = true
consoleImport = "@/hermes"
27 changes: 25 additions & 2 deletions typescript/src/async-rust-call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
return new Promise((resolve) => setTimeout(resolve, delayMs));
}

/**
* This method calls an asynchronous method on the Rust side.
*
Expand Down Expand Up @@ -65,13 +71,30 @@ export async function uniffiRustCallAsync<F, T>(
// The poll, complete and free methods are specialized by the FFIType of the return value.
try {
let pollResult: number | undefined;
do {
while (true) {
// Calling pollFunc with a callback that resolves the promise that pollRust
// returns: pollRust makes the promise, uniffiFutureContinuationCallback resolves it.
pollResult = await pollRust((handle) => {
pollFunc(rustFuture, uniffiFutureContinuationCallback, handle);
});
} while (pollResult != UNIFFI_RUST_FUTURE_POLL_READY);
// 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
if (pollResult == UNIFFI_RUST_FUTURE_POLL_READY) {
break;
}
await delayPromise(100);
}

// Now it's ready, all we need to do is pick up the result (and error).
return liftFunc(
Expand Down

0 comments on commit f657971

Please sign in to comment.