Skip to content

AndroidAppWaker is not sound because AndroidApp/looper does not _actually_ have a 'static lifetime #226

@rib

Description

@rib

While looking at #171, I noticed this separate soundness issue with the current implementation of AndroidAppWaker:

pub struct AndroidAppWaker {
    // The looper pointer is owned by the android_app and effectively
    // has a 'static lifetime, and the ALooper_wake C API is thread
    // safe, so this can be cloned safely and is send + sync safe
    looper: NonNull<ndk_sys::ALooper>,
}

That comment is not the full truth.

Although for most applications an AndroidApp will more-or-less appear like it has a 'static lifetime, it does not actually have a 'static lifetime.

An AndroidApp is passed to android_main when your NativeActivity starts up and will be dropped when your application returns from android_main - but that doesn't necessarily mean your process will exit, since Android's Activity lifecycle is not a process lifecycle and an application may run multiple activities.

Even without considering the theoretical possibility of trying to start more than one NativeActivity or GameActivity per process (difficult for multiple reasons) it would be possible for JNI to call into Rust after your android_main function has returned which could observe an invalid looper pointer if an AndroidAppWaker were stored somewhere with a 'static lifetime.

When we create an AndroidAppWaker we need to call ALooper_acquire() instead of assuming the pointer has a static lifetime.

The Clone implementation for AndroidAppWaker also needs to call ALooper_acquire()

There needs to then be a Drop implementation that call ALooper_release()

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsafetySomething that relates to a safety or soundness issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions