NordSecurity / uniffi-bindgen-go

Uniffi bindings generator for Golang
Mozilla Public License 2.0
75 stars 21 forks source link

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

Closed kegsay closed 6 months ago

kegsay commented 7 months ago

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 https://github.com/NordSecurity/uniffi-bindgen-go/issues/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 commented 6 months ago

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 commented 6 months ago

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.