From 9e600c2d925343520fd9c8449631e75b09b598f1 Mon Sep 17 00:00:00 2001
From: Marijn Suijten <marijns95@gmail.com>
Date: Mon, 16 Oct 2023 18:38:02 +0200
Subject: [PATCH] Bail on errors in log-forwarding thread

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.
---
 android-activity/src/game_activity/mod.rs    | 15 ++++-----
 android-activity/src/native_activity/glue.rs | 35 ++++++++++----------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs
index ef487fa..76609ce 100644
--- a/android-activity/src/game_activity/mod.rs
+++ b/android-activity/src/game_activity/mod.rs
@@ -3,7 +3,7 @@
 use std::collections::HashMap;
 use std::ffi::{CStr, CString};
 use std::fs::File;
-use std::io::{BufRead, BufReader};
+use std::io::{self, BufRead, BufReader};
 use std::marker::PhantomData;
 use std::ops::Deref;
 use std::os::unix::prelude::*;
@@ -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<()> {
             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);
                 }
             }
         });
diff --git a/android-activity/src/native_activity/glue.rs b/android-activity/src/native_activity/glue.rs
index d7014d1..a6b0e66 100644
--- a/android-activity/src/native_activity/glue.rs
+++ b/android-activity/src/native_activity/glue.rs
@@ -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,
@@ -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 = {}",