From 880251c945379889cd2414aa66929770092492e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristupas=20Antanavi=C4=8Dius?= Date: Thu, 14 Mar 2024 11:12:42 +0200 Subject: [PATCH] Fix multiple async packages not working together Future continuation callback is a single global callback on Rust side. Its used to signal bindings from Rust when a future is ready. The problem is if multiple async packages set this callback, only one of these callbacks will be used by Rust code for futures from both packages. The current implementation of this callback simply sends the continuation future result into a channel, but the channel type was using `C.int8_t`, which turns out to be a different type for each package, and channel raises an error when wrong type is being sent in the channel. Luckily this callback is very simple, and it doesn't matter which package's callback is trigerred. So the fix is to use a channel type that is the same for both packages, regular `int8` instead of `C.int8_t`. --- Cargo.lock | 10 +++++++++ bindgen/templates/Async.go | 12 +++++------ binding_tests/issue45_test.go | 21 +++++++++++++++++++ fixtures/Cargo.toml | 1 + fixtures/regressions/issue45/Cargo.toml | 16 ++++++++++++++ fixtures/regressions/issue45/build.rs | 7 +++++++ fixtures/regressions/issue45/src/issue45.udl | 1 + fixtures/regressions/issue45/src/lib.rs | 22 ++++++++++++++++++++ fixtures/src/lib.rs | 1 + 9 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 binding_tests/issue45_test.go create mode 100644 fixtures/regressions/issue45/Cargo.toml create mode 100644 fixtures/regressions/issue45/build.rs create mode 100644 fixtures/regressions/issue45/src/issue45.udl create mode 100644 fixtures/regressions/issue45/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index e39d525..33fca38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1219,6 +1219,7 @@ dependencies = [ "uniffi-fixture-type-limits", "uniffi-go-fixture-destroy", "uniffi-go-fixture-errors", + "uniffi-go-fixture-issue45", "uniffi-go-fixture-name-case", "uniffi-go-fixture-objects", ] @@ -1446,6 +1447,15 @@ dependencies = [ "uniffi_macros", ] +[[package]] +name = "uniffi-go-fixture-issue45" +version = "1.0.0" +dependencies = [ + "uniffi", + "uniffi_build", + "uniffi_macros", +] + [[package]] name = "uniffi-go-fixture-name-case" version = "0.22.0" diff --git a/bindgen/templates/Async.go b/bindgen/templates/Async.go index 55e04b5..d391f28 100644 --- a/bindgen/templates/Async.go +++ b/bindgen/templates/Async.go @@ -3,8 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */#} const ( - uniffiRustFuturePollReady C.int8_t = 0 - uniffiRustFuturePollMaybeReady C.int8_t = 1 + uniffiRustFuturePollReady int8 = 0 + uniffiRustFuturePollMaybeReady int8 = 1 ) func uniffiRustCallAsync( @@ -111,8 +111,8 @@ func uniffiRustCallAsyncInner( pollFunc func(*C.void, unsafe.Pointer, *C.RustCallStatus), freeFunc func(*C.void, *C.RustCallStatus), ) (*C.void, error) { - pollResult := C.int8_t(-1) - waiter := make(chan C.int8_t, 1) + pollResult := int8(-1) + waiter := make(chan int8, 1) chanHandle := cgo.NewHandle(waiter) rustFuture, err := rustCallWithError(converter, func(status *C.RustCallStatus) *C.void { @@ -146,8 +146,8 @@ func uniffiRustCallAsyncInner( //export uniffiFutureContinuationCallback{{ config.package_name.as_ref().unwrap() }} func uniffiFutureContinuationCallback{{ config.package_name.as_ref().unwrap() }}(ptr unsafe.Pointer, pollResult C.int8_t) { doneHandle := *(*cgo.Handle)(ptr) - done := doneHandle.Value().((chan C.int8_t)) - done <- pollResult + done := doneHandle.Value().((chan int8)) + done <- int8(pollResult) } func uniffiInitContinuationCallback() { diff --git a/binding_tests/issue45_test.go b/binding_tests/issue45_test.go new file mode 100644 index 0000000..cbf07ca --- /dev/null +++ b/binding_tests/issue45_test.go @@ -0,0 +1,21 @@ +/* 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/. */ + +package binding_tests + +import ( + "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/issue45" + "testing" + "github.com/stretchr/testify/assert" +) + +// Ensure multiple futures packages work fine together, the other one being +// the "futures" fixture from uniffi-rs. +// https://github.com/NordSecurity/uniffi-bindgen-go/issues/45 + +func TestIssue45(t *testing.T) { + record := issue45.GetAsyncRecord() + assert.Equal(t, record.Id, "foo") + assert.Equal(t, record.Tag, "bar") +} diff --git a/fixtures/Cargo.toml b/fixtures/Cargo.toml index 737263e..960d3d2 100644 --- a/fixtures/Cargo.toml +++ b/fixtures/Cargo.toml @@ -41,3 +41,4 @@ uniffi-go-fixture-destroy = { path = "destroy" } uniffi-go-fixture-errors = { path = "errors" } uniffi-go-fixture-name-case = { path = "name-case" } uniffi-go-fixture-objects = { path = "objects" } +uniffi-go-fixture-issue45 = { path = "regressions/issue45" } diff --git a/fixtures/regressions/issue45/Cargo.toml b/fixtures/regressions/issue45/Cargo.toml new file mode 100644 index 0000000..09fbc11 --- /dev/null +++ b/fixtures/regressions/issue45/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "uniffi-go-fixture-issue45" +version = "1.0.0" +edition = "2021" +publish = false + +[lib] +crate-type = ["lib", "cdylib"] +name = "uniffi_go_issue45" + +[dependencies] +uniffi = {path = "../../../3rd-party/uniffi-rs/uniffi"} +uniffi_macros = {path = "../../../3rd-party/uniffi-rs/uniffi_macros"} + +[build-dependencies] +uniffi_build = {path = "../../../3rd-party/uniffi-rs/uniffi_build", features=["builtin-bindgen"]} diff --git a/fixtures/regressions/issue45/build.rs b/fixtures/regressions/issue45/build.rs new file mode 100644 index 0000000..468fd5f --- /dev/null +++ b/fixtures/regressions/issue45/build.rs @@ -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() { + uniffi_build::generate_scaffolding("./src/issue45.udl").unwrap(); +} diff --git a/fixtures/regressions/issue45/src/issue45.udl b/fixtures/regressions/issue45/src/issue45.udl new file mode 100644 index 0000000..4ddcf6c --- /dev/null +++ b/fixtures/regressions/issue45/src/issue45.udl @@ -0,0 +1 @@ +namespace issue45 {}; diff --git a/fixtures/regressions/issue45/src/lib.rs b/fixtures/regressions/issue45/src/lib.rs new file mode 100644 index 0000000..0d70c23 --- /dev/null +++ b/fixtures/regressions/issue45/src/lib.rs @@ -0,0 +1,22 @@ +/* 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/. */ + +#[derive(uniffi::Record)] +struct Record { + id: String, + tag: String, +} + +// Ensure multiple futures packages work fine together, the other one being +// the "futures" fixture from uniffi-rs. +// https://github.com/NordSecurity/uniffi-bindgen-go/issues/45 +#[uniffi::export] +async fn get_async_record() -> Record { + Record { + id: "foo".to_string(), + tag: "bar".to_string(), + } +} + +include!(concat!(env!("OUT_DIR"), "/issue45.uniffi.rs")); diff --git a/fixtures/src/lib.rs b/fixtures/src/lib.rs index 9ef99dc..fccd3de 100644 --- a/fixtures/src/lib.rs +++ b/fixtures/src/lib.rs @@ -37,6 +37,7 @@ mod uniffi_fixtures { // Go specific uniffi_go_destroy::uniffi_reexport_scaffolding!(); uniffi_go_errors::uniffi_reexport_scaffolding!(); + uniffi_go_issue45::uniffi_reexport_scaffolding!(); uniffi_go_name_case::uniffi_reexport_scaffolding!(); uniffi_go_objects::uniffi_reexport_scaffolding!(); }