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

bug: Cannot use allo-isolate in two libraries with static linking because of symbol name duplication #55

Open
fzyzcjy opened this issue Jan 2, 2024 · 4 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Jan 2, 2024

When a user is using two packages having allo-isolate, and doing static linking, the symbol store_dart_post_cobject is used in two Rust crates, and thus it seems that the linker will throw away one symbol completely. Then, when calling store_dart_post_cobject, only one of the two Rust crates will actually store that pointer. The other crate will never store it, and thus allo-isolate cannot work at all.

/cc @AlexV525

@shekohex
Copy link
Owner

shekohex commented Jan 2, 2024

How is it possible to have two allo-isolate in a single binary? It should only be one! Unless we have two different versions and cargo decided to link both? Which in this case is bad! I would recommend dedup Cargo.lock and find out which crate that uses the older version of allo-isolate and update it, should be only one if they both have same version.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Jan 2, 2024

How is it possible to have two allo-isolate in a single binary?

  • The app uses Dart lib A and lib B
  • lib A uses a rust crate C. Thus crate C (and its dependency allo-isolate) are compiled into library_c.a and statically links to the final app
  • lib B similarly use crate D, and library_d.a

In this case we have trouble...

That said, Flutter's official template provide dynamic linking now (IIRC years ago it was static linking), so may not be a problem, unless users choose to do static linking.

@shekohex
Copy link
Owner

shekohex commented Jan 2, 2024

That said, Flutter's official template provide dynamic linking now (IIRC years ago it was static linking), so may not be a problem, unless users choose to do static linking.

iOS is always static linked, no?

But I hear you, I'm open to other ideas to set up the bridge through FFI, maybe each library author would choose their own exposed function? Through a macro, for example?

Like the following:

// in allo-isolate code lib.rs
#[macro_export]
macro_rules! mk_store_dart_post_cobject {
	($name:ident) => {
		#[no_mangle]
		pub unsafe extern "C" fn $name(ptr: ::allo_isolate::ffi::DartPostCObjectFnType) {
			::allo_isolate::POST_COBJECT.store(Some($ptr), Ordering::Relaxed);
		}
	},
}
// in my-awesome-crate lib.rs
allo_isolate::mk_store_dart_post_cobject!(my_awesome_crate_store_dart_post_cobject);

This way, we will not have a duplicated symbols.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Jan 2, 2024

iOS is always static linked, no?

I also did that, but seems no today, which is surprising. -> https://github.com/flutter/flutter/blob/8b6277e63868c2029f1e2327879b7899be44fbe2/packages/flutter_tools/templates/plugin_ffi/lib/projectName.dart.tmpl#L47-L58

Through a macro, for example?

It sounds like an option.
For FRB, a solution is to create a (generated) wrapper function.

Anyway, as mentioned above, this is not a problem for the dynamic linking now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants