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

Weird/UB ndk APIs found by removing NIHisms in android-activity #478

Open
MarijnS95 opened this issue Jul 29, 2024 · 0 comments
Open

Weird/UB ndk APIs found by removing NIHisms in android-activity #478

MarijnS95 opened this issue Jul 29, 2024 · 0 comments

Comments

@MarijnS95
Copy link
Member

MarijnS95 commented Jul 29, 2024

android-activity open-codes a lot of ndk-provided API, directly on top of native functions and symbols from ndk-sys. While attempting to remove most of it to simplify and clear up the implementation with safer Rust, I found that the ndk could do some things quite a bit better, even if only by documenting expectations. Perhaps this is why certain things were open-coded in the first place, besides being ported from raw C code.

Since the ndk crate is quite old by now, and has seen a lot of "Just Make it Work™", things like lifetime management and correctness when it comes to safety/UB have not always been kept in mind while writing bindings. Instead of creating a lot of separate issues (or PRs) right now, I am using this issue as a public scratch-pad to keep track of questions and ideas that need to tracked and looked into later.

  • NativeActivity returns &Paths from raw pointers: could these ever be null()/None?
  • NativeActivity::asset_manager() has no lifetime: this should be bound to &self or is it "Application-global"?
  • InputQueue has no lifetime and no Drop/Clone: document its lifetime guarantees and return borrows where necessary?
    Its lifetime is tied to the onInputQueueDestroyed callback whose docs describe that:

    You should no longer try to reference this object upon returning from this function.

  • ForeignLooper (and various other structures, see Unify lifetime handling for NativeWindow and HardwareBuffer #309) cannot be created without lifetime management, i.e. to temporarily borrow/handle the pointer safely while it is valid inside a callback.
    For lack of unsafe fn borrow_from_ptr(NonNull...) -> &'unknown Self (impossible to write in Rust anyway), it seems most convenient to call let looper = ManuallyDrop::new(ForeignLooper::from_ptr(looper)); now.
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

No branches or pull requests

1 participant