MotionEvent + KeyEvent from ndk crate shouldn't be exposed without a lifetime
rib opened this issue · 1 comments
When considering how to address #40, (where it makes sense to want to introduce a lifetime for the InputEvent enum) it occurred to me that it was also not good that (with the NativeActivity backend) the existing API exposes the MotionEvent and KeyEvent types from the ndk crate without any associated lifetime that would stop applications keeping copies of the events for too long and risk dereferencing invalid pointers.
Since android-activity only exposes input events by reference within input_events() callbacks we can simply add our own wrappers for MotionEvent and KeyEvent with suitable lifetime parameters that will block code from keeping these pointers too long.
I guess I'll needing to mirror the ndk events API explicitly (similar to what is already done for GameActivity) to avoid publicly exposing the ndk types directly. (as a side-effect this should result in better compatibility since it will be more consistent with the GameActivity backend)
Fairly sure this can be addressed without (practically) changing the public API, so we can address for the 0.4 release.
Originally posted by @rib in https://github.com/rib/android-activity/issues/39#issuecomment-1311925732
Referencing back to fab31d3#commitcomment-107316896, I think the ndk types are pretty unsafe here by carrying a raw pointer with no idea whatsoever how long it remains valid. The Android docs don't specify anything either so our best bet is to give it a borrow/lifetime on InputQueue and make get_event() take &self mut so that reading the next event (assuming it could invalidate the previous event pointer) is only allowed to happen if the previous InputEvent was dropped.