Generalize log-forwarding setup and stop thread on IO errors

When `read_line()` starts returning `Err` the current `if let Ok`
condition ignores those, likely causing the `loop` to spin indefinitely
while this function keeps returning errors.

Note that we don't currently store the join handle for this thread
anywhere, so won't see the error surface either (just like how the join
handle for the main thread is never checked).  Perhaps we should call
`log::error!()` to make the user aware that their IO logging has
mysteriously terminated.
This commit is contained in:
Marijn Suijten
2023-10-16 18:38:02 +02:00
parent 98aef99419
commit af341897a2
3 changed files with 55 additions and 73 deletions
+8 -38
View File
@@ -1,21 +1,17 @@
#![cfg(feature = "game-activity")]
use std::collections::HashMap;
use std::ffi::{CStr, CString};
use std::fs::File;
use std::io::{BufRead, BufReader};
use std::marker::PhantomData;
use std::ops::Deref;
use std::os::unix::prelude::*;
use std::panic::catch_unwind;
use std::ptr;
use std::ptr::NonNull;
use std::sync::Weak;
use std::sync::{Arc, Mutex, RwLock};
use std::time::Duration;
use std::{ptr, thread};
use libc::c_void;
use log::{error, trace, Level};
use log::{error, trace};
use jni_sys::*;
@@ -29,9 +25,9 @@ use ndk::native_window::NativeWindow;
use crate::error::InternalResult;
use crate::input::{Axis, KeyCharacterMap, KeyCharacterMapBinding};
use crate::jni_utils::{self, CloneJavaVM};
use crate::util::{abort_on_panic, android_log, log_panic};
use crate::util::{abort_on_panic, forward_stdio_to_logcat, log_panic, try_get_path_from_ptr};
use crate::{
util, AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags,
AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags,
};
mod ffi;
@@ -617,21 +613,21 @@ impl AndroidAppInner {
pub fn internal_data_path(&self) -> Option<std::path::PathBuf> {
unsafe {
let app_ptr = self.native_app.as_ptr();
util::try_get_path_from_ptr((*(*app_ptr).activity).internalDataPath)
try_get_path_from_ptr((*(*app_ptr).activity).internalDataPath)
}
}
pub fn external_data_path(&self) -> Option<std::path::PathBuf> {
unsafe {
let app_ptr = self.native_app.as_ptr();
util::try_get_path_from_ptr((*(*app_ptr).activity).externalDataPath)
try_get_path_from_ptr((*(*app_ptr).activity).externalDataPath)
}
}
pub fn obb_path(&self) -> Option<std::path::PathBuf> {
unsafe {
let app_ptr = self.native_app.as_ptr();
util::try_get_path_from_ptr((*(*app_ptr).activity).obbPath)
try_get_path_from_ptr((*(*app_ptr).activity).obbPath)
}
}
}
@@ -913,33 +909,7 @@ extern "Rust" {
#[no_mangle]
pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) {
abort_on_panic(|| {
// Maybe make this stdout/stderr redirection an optional / opt-in feature?...
let file = {
let mut logpipe: [RawFd; 2] = Default::default();
libc::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC);
libc::dup2(logpipe[1], libc::STDOUT_FILENO);
libc::dup2(logpipe[1], libc::STDERR_FILENO);
libc::close(logpipe[1]);
File::from_raw_fd(logpipe[0])
};
thread::spawn(move || {
let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
let mut reader = BufReader::new(file);
let mut buffer = String::new();
loop {
buffer.clear();
if let Ok(len) = reader.read_line(&mut buffer) {
if len == 0 {
break;
} else if let Ok(msg) = CString::new(buffer.clone()) {
android_log(Level::Info, tag, &msg);
}
}
}
});
let _join_log_forwarder = forward_stdio_to_logcat();
let jvm = unsafe {
let jvm = (*(*native_app).activity).vm;
+2 -33
View File
@@ -3,23 +3,17 @@
//! synchronization between the two threads.
use std::{
ffi::{CStr, CString},
fs::File,
io::{BufRead, BufReader},
ops::Deref,
os::unix::prelude::{FromRawFd, RawFd},
panic::catch_unwind,
ptr::{self, NonNull},
sync::{Arc, Condvar, Mutex, Weak},
};
use log::Level;
use ndk::{configuration::Configuration, input_queue::InputQueue, native_window::NativeWindow};
use crate::{
jni_utils::CloneJavaVM,
util::android_log,
util::{abort_on_panic, log_panic},
util::{abort_on_panic, forward_stdio_to_logcat, log_panic},
ConfigurationRef,
};
@@ -834,32 +828,7 @@ extern "C" fn ANativeActivity_onCreate(
saved_state_size: libc::size_t,
) {
abort_on_panic(|| {
// Maybe make this stdout/stderr redirection an optional / opt-in feature?...
let file = unsafe {
let mut logpipe: [RawFd; 2] = Default::default();
libc::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC);
libc::dup2(logpipe[1], libc::STDOUT_FILENO);
libc::dup2(logpipe[1], libc::STDERR_FILENO);
libc::close(logpipe[1]);
File::from_raw_fd(logpipe[0])
};
std::thread::spawn(move || {
let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
let mut reader = BufReader::new(file);
let mut buffer = String::new();
loop {
buffer.clear();
if let Ok(len) = reader.read_line(&mut buffer) {
if len == 0 {
break;
} else if let Ok(msg) = CString::new(buffer.clone()) {
android_log(Level::Info, tag, &msg);
}
}
}
});
let _join_log_forwarder = forward_stdio_to_logcat();
log::trace!(
"Creating: {:p}, saved_state = {:p}, save_state_size = {}",
+45 -2
View File
@@ -1,7 +1,12 @@
use log::Level;
use log::{error, Level};
use std::{
ffi::{CStr, CString},
os::raw::c_char,
fs::File,
io::{BufRead as _, BufReader, Result},
os::{
fd::{FromRawFd as _, RawFd},
raw::c_char,
},
};
pub fn try_get_path_from_ptr(path: *const c_char) -> Option<std::path::PathBuf> {
@@ -31,6 +36,44 @@ pub(crate) fn android_log(level: Level, tag: &CStr, msg: &CStr) {
}
}
pub(crate) fn forward_stdio_to_logcat() -> std::thread::JoinHandle<Result<()>> {
// XXX: make this stdout/stderr redirection an optional / opt-in feature?...
let file = unsafe {
let mut logpipe: [RawFd; 2] = Default::default();
libc::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC);
libc::dup2(logpipe[1], libc::STDOUT_FILENO);
libc::dup2(logpipe[1], libc::STDERR_FILENO);
libc::close(logpipe[1]);
File::from_raw_fd(logpipe[0])
};
std::thread::Builder::new()
.name("stdio-to-logcat".to_string())
.spawn(move || -> Result<()> {
let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
let mut reader = BufReader::new(file);
let mut buffer = String::new();
loop {
buffer.clear();
let len = match reader.read_line(&mut buffer) {
Ok(len) => len,
Err(e) => {
error!("Logcat forwarder failed to read stdin/stderr: {e:?}");
break Err(e);
}
};
if len == 0 {
break Ok(());
} else if let Ok(msg) = CString::new(buffer.clone()) {
android_log(Level::Info, tag, &msg);
}
}
})
.expect("Failed to start stdout/stderr to logcat forwarder thread")
}
pub(crate) fn log_panic(panic: Box<dyn std::any::Any + Send>) {
let rust_panic = unsafe { CStr::from_bytes_with_nul_unchecked(b"RustPanic\0") };