Skip to content

Commit

Permalink
Bail on errors in log-forwarding thread
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MarijnS95 committed Oct 16, 2023
1 parent a291e37 commit 9e600c2
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 26 deletions.
15 changes: 7 additions & 8 deletions android-activity/src/game_activity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::collections::HashMap;
use std::ffi::{CStr, CString};

Check failure on line 4 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (1.68.0)

unused imports: `CStr`, `CString`

Check failure on line 4 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (stable)

unused imports: `CStr`, `CString`
use std::fs::File;

Check failure on line 5 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (1.68.0)

unused import: `std::fs::File`

Check failure on line 5 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (stable)

unused import: `std::fs::File`
use std::io::{BufRead, BufReader};
use std::io::{self, BufRead, BufReader};

Check failure on line 6 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (1.68.0)

unused imports: `BufRead`, `BufReader`, `self`

Check failure on line 6 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (stable)

unused imports: `BufRead`, `BufReader`, `self`
use std::marker::PhantomData;
use std::ops::Deref;
use std::os::unix::prelude::*;
Expand Down Expand Up @@ -916,19 +916,18 @@ pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) {
libc::pipe(logpipe.as_mut_ptr());
libc::dup2(logpipe[1], libc::STDOUT_FILENO);
libc::dup2(logpipe[1], libc::STDERR_FILENO);
thread::spawn(move || {
thread::spawn(move || io::Result<()> {

Check failure on line 919 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (1.68.0)

comparison operators cannot be chained

Check failure on line 919 in android-activity/src/game_activity/mod.rs

View workflow job for this annotation

GitHub Actions / build (stable)

comparison operators cannot be chained
let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
let file = File::from_raw_fd(logpipe[0]);
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 len = reader.read_line(&mut buffer)?;
if len == 0 {
break Ok(());
} else if let Ok(msg) = CString::new(buffer.clone()) {
android_log(Level::Info, tag, &msg);
}
}
});
Expand Down
35 changes: 17 additions & 18 deletions android-activity/src/native_activity/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use std::{
ffi::{CStr, CString},
fs::File,
io::{BufRead, BufReader},
io::{self, BufRead, BufReader},
ops::Deref,
os::unix::prelude::{FromRawFd, RawFd},
panic::catch_unwind,
Expand Down Expand Up @@ -835,28 +835,27 @@ extern "C" fn ANativeActivity_onCreate(
) {
abort_on_panic(|| {
// Maybe make this stdout/stderr redirection an optional / opt-in feature?...
unsafe {
let file = unsafe {
let mut logpipe: [RawFd; 2] = Default::default();
libc::pipe(logpipe.as_mut_ptr());
libc::dup2(logpipe[1], libc::STDOUT_FILENO);
libc::dup2(logpipe[1], libc::STDERR_FILENO);
std::thread::spawn(move || {
let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
let file = File::from_raw_fd(logpipe[0]);
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);
}
}
File::from_raw_fd(logpipe[0])
};
std::thread::spawn(move || -> io::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 = reader.read_line(&mut buffer)?;
if len == 0 {
break Ok(());
} else if let Ok(msg) = CString::new(buffer.clone()) {
android_log(Level::Info, tag, &msg);
}
});
}
}
});

log::trace!(
"Creating: {:p}, saved_state = {:p}, save_state_size = {}",
Expand Down

0 comments on commit 9e600c2

Please sign in to comment.