Skip to content

Commit

Permalink
Fix multiple async packages not working together
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
arg0d committed Mar 14, 2024
1 parent 3862b3a commit 880251c
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 6 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions bindgen/templates/Async.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down
21 changes: 21 additions & 0 deletions binding_tests/issue45_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
1 change: 1 addition & 0 deletions fixtures/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
16 changes: 16 additions & 0 deletions fixtures/regressions/issue45/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"]}
7 changes: 7 additions & 0 deletions fixtures/regressions/issue45/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() {
uniffi_build::generate_scaffolding("./src/issue45.udl").unwrap();
}
1 change: 1 addition & 0 deletions fixtures/regressions/issue45/src/issue45.udl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
namespace issue45 {};
22 changes: 22 additions & 0 deletions fixtures/regressions/issue45/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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"));
1 change: 1 addition & 0 deletions fixtures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
}

0 comments on commit 880251c

Please sign in to comment.