Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It's not possible to have 2 libraries which use async functions #45

Closed
kegsay opened this issue Mar 8, 2024 · 2 comments · Fixed by #46
Closed

It's not possible to have 2 libraries which use async functions #45

kegsay opened this issue Mar 8, 2024 · 2 comments · Fixed by #46

Comments

@kegsay
Copy link
Contributor

kegsay commented Mar 8, 2024

Apply this patch to the errors fixture:

diff --git a/fixtures/errors/src/lib.rs b/fixtures/errors/src/lib.rs
index 2c60467..990bf26 100644
--- a/fixtures/errors/src/lib.rs
+++ b/fixtures/errors/src/lib.rs
@@ -165,4 +165,10 @@ fn error_timestamp() -> Result<std::time::SystemTime, BoobyTrapError> {
     Err(BoobyTrapError::IceSlip)
 }
 
+// Cannot use UDL here: see https://github.com/mozilla/uniffi-rs/issues/1915
+#[uniffi::export]
+async fn async_error() -> Result<(), BoobyTrapError> {
+    Ok(())
+}
+
 include!(concat!(env!("OUT_DIR"), "/errors.uniffi.rs"));

Then ./build.sh, ./build_bindings.sh and finally ./test_bindings.sh:

--- FAIL: TestFuturesAlwaysReady (0.00s)
panic: interface conversion: interface {} is chan futures._Ctype_schar, not chan errors._Ctype_schar [recovered]
	panic: interface conversion: interface {} is chan futures._Ctype_schar, not chan errors._Ctype_schar

goroutine 29 [running]:
testing.tRunner.func1.2({0x105153580, 0x140000c2090})
	/usr/local/go/src/testing/testing.go:1631 +0x1c4
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1634 +0x33c
panic({0x105153580?, 0x140000c2090?})
	/usr/local/go/src/runtime/panic.go:770 +0x124
github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/errors.uniffiFutureContinuationCallbackerrors(0x10?, 0x0)
	.../uniffi-bindgen-go/binding_tests/generated/errors/errors.go:1824 +0x78
github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/futures._Cfunc_ffi_uniffi_futures_rust_future_poll_i8(0x600003bf00c0, 0x140000c4048, 0x1400000e1f8)
	_cgo_gotypes.go:263 +0x30
github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/futures.AlwaysReady.func2.1(0x600003bf00c0, 0x140000c4048, 0x1400000e1f8)
	.../uniffi-bindgen-go/binding_tests/generated/futures/futures.go:1318 +0x8c

The panic is in this function:

func uniffiFutureContinuationCallbackerrors(ptr unsafe.Pointer, pollResult C.int8_t) {
	doneHandle := *(*cgo.Handle)(ptr)
	done := doneHandle.Value().((chan C.int8_t))
	done <- pollResult
}

The problem is similar to #43 in that when the runtime type cast is made, it fails because the callback is called with C.int8_t from the futures package, whereas this is type-casting to C.int8_t from the errors package.

In reality, the callback should not be triggered at all from the errors package (as the diff doesn't actually call the defined function anywhere) but this happens because it is set when the package is loaded:

func init() {
	uniffiInitContinuationCallback()
	uniffiCheckChecksums()
}

This init function appears in both the errors and futures package, and I guess they step on each other (errors runs first so I guess the 2nd call to uniffiInitContinuationCallback() is a no-op, causing the errors callback to be called for futures tests.

@arg0d
Copy link
Contributor

arg0d commented Mar 12, 2024

Yikes, this is gnarly. Seems like ffi_rust_future_continuation_callback_set uses global state, not per-package state. I'm not sure whats the best way to move forward with this. I believe starting with uniffi-rs:v0.26.0 futures implementation was revamped with poll-based instead of callback-based architecture, and uniffiInitContinuationCallback is not needed anymore. I doubt Mozilla upstream guys would be willing to somehow revamp futures implementation for uniffi-rs:v0.25.0 to make it compatible with multiple ffi_rust_future_continuation_callback_set calls. I guess the "easiest" solution would be to somehow ensure only 1 bindings package invokes uniffiInitContinuationCallback.

@arg0d
Copy link
Contributor

arg0d commented Mar 13, 2024

I'm afraid I can't provide much assistance with this at the moment, async is not a priority for our teams and I'm currently focusing on other work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants