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.

Sign up for GitHub

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

fix: handle JNI java exceptions properly in client_core #2563

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: handle JNI java exceptions properly in client_core #2563

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 130 additions & 95 deletions alvr/system_info/src/android.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,45 @@
use alvr_common::warn;
use jni::{objects::JObject, sys::jobject, JNIEnv, JavaVM};
use jni::{
errors::Error,
objects::{JObject, JString, JValue, JValueOwned},
sys::jobject,
JNIEnv, JavaVM,
};
use std::net::{IpAddr, Ipv4Addr};

pub const MICROPHONE_PERMISSION: &str = "android.permission.RECORD_AUDIO";

// Will call a JNI method and handle any Java exceptions that might occur
fn env_call_method_handled<'local>(
env: &mut JNIEnv<'local>,
object: &JObject,
method: &str,
sig: &str,
args: &[JValue],
) -> Result<JValueOwned<'local>, Error> {
Comment on lines +13 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually i follow the path of fewer parameters when making a function to abstract away some code. In this case it would be much easier to make a function taking a jni::Result<T> and return a anyhow::Result<T>. Inside the function, don't panic because you don't know the intention of the caller, even if we always unwrap, don't expect anything. Instead limit this function to just transform the error.

Actually, before doing this, check if just calling .to_any() at the caller site is enough (with the purpose of making the error message clearer). This function is defined in alvr_common. Other things you could do is to play with different printing methods (using {} or {:?} placeholders with format!())

env.call_method(object, method, sig, args)
.map_err(|e| match e {
jni::errors::Error::JavaException => {
env.exception_describe().unwrap();
let jthorw = env.exception_occurred().unwrap();
env.exception_clear().unwrap();
let exception_to_jstring: JString = env
.call_method(jthorw, "toString", "()Ljava/lang/String;", &[])
.unwrap()
.l()
.unwrap()
.into();
let excp_string = env.get_string(&exception_to_jstring).unwrap();
let err = String::from("Java Exception Occured: ") + excp_string.to_str().unwrap();
// At this point even if we let this error propagate, the next unwrap will panic with only unclear "JavaException" literal,
// so we can panic here itself with more detailed JavaException reason message and let crate::backtrace::backtrace() be printed
// This can also be synced with server side in future if we have that feature
panic!("{err}");
}
_ => e,
})
}

pub fn vm() -> JavaVM {
unsafe { JavaVM::from_raw(ndk_context::android_context().vm().cast()).unwrap() }
}
Expand All @@ -28,25 +64,26 @@ pub fn try_get_permission(permission: &str) {

let mic_perm_jstring = env.new_string(permission).unwrap();

let permission_status = env
.call_method(
unsafe { JObject::from_raw(context()) },
"checkSelfPermission",
"(Ljava/lang/String;)I",
&[(&mic_perm_jstring).into()],
)
.unwrap()
.i()
.unwrap();
let permission_status = env_call_method_handled(
&mut env,
unsafe { &JObject::from_raw(context()) },
"checkSelfPermission",
"(Ljava/lang/String;)I",
&[(&mic_perm_jstring).into()],
)
.unwrap()
.i()
.unwrap();

if permission_status != 0 {
let string_class = env.find_class("java/lang/String").unwrap();
let perm_array = env
.new_object_array(1, string_class, mic_perm_jstring)
.unwrap();

env.call_method(
unsafe { JObject::from_raw(context()) },
env_call_method_handled(
&mut env,
unsafe { &JObject::from_raw(context()) },
"requestPermissions",
"([Ljava/lang/String;I)V",
&[(&perm_array).into(), 0.into()],
Expand Down Expand Up @@ -86,8 +123,9 @@ pub fn manufacturer_name() -> String {
fn get_system_service<'a>(env: &mut JNIEnv<'a>, service_name: &str) -> JObject<'a> {
let service_str = env.new_string(service_name).unwrap();

env.call_method(
unsafe { JObject::from_raw(context()) },
env_call_method_handled(
env,
unsafe { &JObject::from_raw(context()) },
"getSystemService",
"(Ljava/lang/String;)Ljava/lang/Object;",
&[(&service_str).into()],
Expand All @@ -103,18 +141,17 @@ pub fn local_ip() -> IpAddr {
let mut env = vm.attach_current_thread().unwrap();

let wifi_manager = get_system_service(&mut env, "wifi");
let wifi_info = env
.call_method(
wifi_manager,
"getConnectionInfo",
"()Landroid/net/wifi/WifiInfo;",
&[],
)
.unwrap()
.l()
.unwrap();
let ip_i32 = env
.call_method(wifi_info, "getIpAddress", "()I", &[])
let wifi_info = env_call_method_handled(
&mut env,
&wifi_manager,
"getConnectionInfo",
"()Landroid/net/wifi/WifiInfo;",
&[],
)
.unwrap()
.l()
.unwrap();
let ip_i32 = env_call_method_handled(&mut env, &wifi_info, "getIpAddress", "()I", &[])
.unwrap()
.i()
.unwrap();
Expand All @@ -133,18 +170,17 @@ pub fn set_wifi_lock(enabled: bool) {
let wifi_manager = get_system_service(&mut env, "wifi");

fn set_lock<'a>(env: &mut JNIEnv<'a>, lock: &JObject, enabled: bool) {
env.call_method(lock, "setReferenceCounted", "(Z)V", &[false.into()])
.unwrap();
env.call_method(
&lock,
env_call_method_handled(env, lock, "setReferenceCounted", "(Z)V", &[false.into()]).unwrap();
env_call_method_handled(
env,
lock,
if enabled { "acquire" } else { "release" },
"()V",
&[],
)
.unwrap();

let lock_is_aquired = env
.call_method(lock, "isHeld", "()Z", &[])
let lock_is_aquired = env_call_method_handled(env, &lock, "isHeld", "()Z", &[])
.unwrap()
.z()
.unwrap();
Expand All @@ -155,38 +191,38 @@ pub fn set_wifi_lock(enabled: bool) {
}

let wifi_lock_jstring = env.new_string("alvr_wifi_lock").unwrap();
let wifi_lock = env
.call_method(
&wifi_manager,
"createWifiLock",
"(ILjava/lang/String;)Landroid/net/wifi/WifiManager$WifiLock;",
&[
if get_api_level() >= 29 {
// Recommended for virtual reality since it disables WIFI scans
4 // WIFI_MODE_FULL_LOW_LATENCY
} else {
3 // WIFI_MODE_FULL_HIGH_PERF
}
.into(),
(&wifi_lock_jstring).into(),
],
)
.unwrap()
.l()
.unwrap();
let wifi_lock = env_call_method_handled(
&mut env,
&wifi_manager,
"createWifiLock",
"(ILjava/lang/String;)Landroid/net/wifi/WifiManager$WifiLock;",
&[
if get_api_level() >= 29 {
// Recommended for virtual reality since it disables WIFI scans
4 // WIFI_MODE_FULL_LOW_LATENCY
} else {
3 // WIFI_MODE_FULL_HIGH_PERF
}
.into(),
(&wifi_lock_jstring).into(),
],
)
.unwrap()
.l()
.unwrap();
set_lock(&mut env, &wifi_lock, enabled);

let multicast_lock_jstring = env.new_string("alvr_multicast_lock").unwrap();
let multicast_lock = env
.call_method(
wifi_manager,
"createMulticastLock",
"(Ljava/lang/String;)Landroid/net/wifi/WifiManager$MulticastLock;",
&[(&multicast_lock_jstring).into()],
)
.unwrap()
.l()
.unwrap();
let multicast_lock = env_call_method_handled(
&mut env,
&wifi_manager,
"createMulticastLock",
"(Ljava/lang/String;)Landroid/net/wifi/WifiManager$MulticastLock;",
&[(&multicast_lock_jstring).into()],
)
.unwrap()
.l()
.unwrap();
set_lock(&mut env, &multicast_lock, enabled);
}

Expand All @@ -204,9 +240,8 @@ pub fn get_battery_status() -> (f32, bool) {
&[(&intent_action_jstring).into()],
)
.unwrap();
let battery_intent = env
.call_method(
unsafe { JObject::from_raw(context()) },
let battery_intent = env_call_method_handled(&mut env,
unsafe { &JObject::from_raw(context()) },
"registerReceiver",
"(Landroid/content/BroadcastReceiver;Landroid/content/IntentFilter;)Landroid/content/Intent;",
&[(&JObject::null()).into(), (&intent_filter).into()],
Expand All @@ -216,39 +251,39 @@ pub fn get_battery_status() -> (f32, bool) {
.unwrap();

let level_jstring = env.new_string("level").unwrap();
let level = env
.call_method(
&battery_intent,
"getIntExtra",
"(Ljava/lang/String;I)I",
&[(&level_jstring).into(), (-1).into()],
)
.unwrap()
.i()
.unwrap();
let level = env_call_method_handled(
&mut env,
&battery_intent,
"getIntExtra",
"(Ljava/lang/String;I)I",
&[(&level_jstring).into(), (-1).into()],
)
.unwrap()
.i()
.unwrap();
let scale_jstring = env.new_string("scale").unwrap();
let scale = env
.call_method(
&battery_intent,
"getIntExtra",
"(Ljava/lang/String;I)I",
&[(&scale_jstring).into(), (-1).into()],
)
.unwrap()
.i()
.unwrap();
let scale = env_call_method_handled(
&mut env,
&battery_intent,
"getIntExtra",
"(Ljava/lang/String;I)I",
&[(&scale_jstring).into(), (-1).into()],
)
.unwrap()
.i()
.unwrap();

let plugged_jstring = env.new_string("plugged").unwrap();
let plugged = env
.call_method(
&battery_intent,
"getIntExtra",
"(Ljava/lang/String;I)I",
&[(&plugged_jstring).into(), (-1).into()],
)
.unwrap()
.i()
.unwrap();
let plugged = env_call_method_handled(
&mut env,
&battery_intent,
"getIntExtra",
"(Ljava/lang/String;I)I",
&[(&plugged_jstring).into(), (-1).into()],
)
.unwrap()
.i()
.unwrap();

(level as f32 / scale as f32, plugged > 0)
}
Loading