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

Empty Android context #2466

Open
Sitin opened this issue Dec 23, 2024 · 10 comments
Open

Empty Android context #2466

Sitin opened this issue Dec 23, 2024 · 10 comments
Labels
awaiting Waiting for responses, PR, further discussions, upstream release, etc bug Something isn't working

Comments

@Sitin
Copy link

Sitin commented Dec 23, 2024

Describe the bug

I've configured Android context as it was described in the docs (Method 1). Still, the context returned by ndk_context::android_context() is null.

Steps to reproduce

Create a project and set up Android context as descibed in the docs (Method 1).

Create a simple function that discovers the current android context:

#[flutter_rust_bridge::frb(sync)]
pub fn debug_android_context() {
    #[cfg(target_os = "android")]
    {
        let ctx = ndk_context::android_context();
        log::warn!("Android context: {:?}", ctx);
    }
}

Call it from Flutter.

You will see that context is null (0x0):

W/my_broken_lib::api..( 6780): Android context: AndroidContext { java_vm: 0xb40000724dc59540, context_jobject: 0x0 }

Logs

W/my_broken_lib::api..( 6780): Android context: AndroidContext { java_vm: 0xb40000724dc59540, context_jobject: 0x0 }


### Expected behavior

The context should be initialised.

### Generated binding code

_No response_

### OS

Android

### Version of `flutter_rust_bridge_codegen`

2.6.0

### Flutter info

_No response_

### Version of `clang++`

_No response_

### Additional context

_No response_
@Sitin Sitin added the bug Something isn't working label Dec 23, 2024
Copy link

welcome bot commented Dec 23, 2024

Hi! Thanks for opening your first issue here! 😄

@fzyzcjy
Copy link
Owner

fzyzcjy commented Dec 24, 2024

This part seems to be contributed by @powpingdone. Thus cc @powpingdone - do you have any suggestions? :)

@fzyzcjy fzyzcjy added the awaiting Waiting for responses, PR, further discussions, upstream release, etc label Dec 24, 2024
@powpingdone
Copy link
Contributor

Thanks fzyz.

@Sitin Check in the JNI_onload res parameter. Is that null?

@Sitin
Copy link
Author

Sitin commented Dec 24, 2024

It seems that logger is not configured at this stage and I only can fail by returning jni::JNIVersion::Invalid. How should I check his pointer for being null?

@powpingdone
Copy link
Contributor

Use https://crates.io/crates/android_logger

@Sitin
Copy link
Author

Sitin commented Dec 24, 2024

The res is indeed null.

I forgot to mention that I've already used android_logger but it seems that the logger can't be initialized at this stage. But there is a trick:

#[cfg(target_os = "android")]
#[no_mangle]
pub extern "C" fn JNI_OnLoad(vm: JavaVM, res: *mut std::os::raw::c_void) -> jni::sys::jint {
    use std::ffi::c_void;

    log::warn!("JNI_OnLoad called");

    if res.is_null() {
        return jni::sys::JNI_ERR;
    }

    let vm = vm.get_java_vm_pointer() as *mut c_void;
    unsafe {
        ndk_context::initialize_android_context(vm, res);
    }

    jni::JNIVersion::V6.into()
}

@tmpfs
Copy link
Contributor

tmpfs commented Jan 7, 2025

I can reproduce this and think that Method 1 from the docs should be removed; the only way I've managed to correctly initialize the context is with method 2; from reading elsewhere, it seems that onAttachedToEngine() is the recommended way to access the context in Flutter.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 7, 2025

Feel free to update doc!

@tmpfs
Copy link
Contributor

tmpfs commented Jan 7, 2025

@fzyzcjy, yes, the docs definitely need updating, in Method 2, the most important part (initializing the NDK context on the Rust side is not shown).

Having just debugged this there are a couple of important points to get this to work.

  1. The JObject reference to the Context must be converted to a GlobalRef to prevent GC (using JNIEnv::new_global_ref())
  2. The GlobalRef must be kept alive in the Rust code

Here is how I ended up implementing init_android (will need changing to match the namespace and plugin name on the Kotlin side):

#[cfg(target_os = "android")]
mod init_android_context {
    use jni::{objects::JClass, objects::JObject, objects::GlobalRef, JNIEnv};
    use std::sync::OnceLock;
    use std::ffi::c_void;

    static CTX: OnceLock<GlobalRef> = OnceLock::new();

    #[no_mangle]
    pub extern "system" fn Java_com_example_local_1auth_MyPlugin_init_1android(
        env: JNIEnv,
        _class: JClass,
        ctx: JObject,
    ) {
        let global_ref = env.new_global_ref(&ctx).expect("to make global reference");
        let vm = env.get_java_vm().unwrap();
        let vm = vm.get_java_vm_pointer() as *mut c_void;
        unsafe {
            ndk_context::initialize_android_context(vm, global_ref.as_obj().as_raw() as _);
        }
        CTX.get_or_init(|| global_ref);
    }
}

Also I think on the Kotlin side it makes sense to add the plugin before calling super.configureFlutterEngine():

class MainActivity : FlutterActivity() {
    override fun configureFlutterEngine(
        @NonNull flutterEngine: FlutterEngine,
    ) {
        flutterEngine.plugins.add(MyPlugin())
        super.configureFlutterEngine(flutterEngine)
    }
}

Perhaps other people could verify this fixes android context initialization issues before we update the docs to be certain.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 7, 2025

Looks great! Feel free to PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting Waiting for responses, PR, further discussions, upstream release, etc bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants