From 707a9d3d5069d632456e4985396b8b2dca40f539 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Wed, 16 Aug 2023 00:59:59 +0200 Subject: [PATCH] Close `logpipe` input file descriptor after `dup2()` When the input file descriptor of the `pipe()` is `dup2()`'d into `stdin` and `stdout` it is effectively copied, leaving the original file descriptor open and leaking at the end of these statements. Only the output file descriptor has its ownership transferred to `File` and will be cleaned up properly. This should cause the reading end to read EOF and return zero bytes when `stdin` and `stdout` is open, rather than remaining open "indefinitely" (barring the whole process being taken down) as there will always be that one file descriptor referencing the input end of the pipe. --- android-activity/src/game_activity/mod.rs | 16 ++++++--- android-activity/src/native_activity/glue.rs | 37 +++++++++++--------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs index ef487fa..5809e9b 100644 --- a/android-activity/src/game_activity/mod.rs +++ b/android-activity/src/game_activity/mod.rs @@ -912,13 +912,19 @@ extern "Rust" { 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 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); + + 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 file = File::from_raw_fd(logpipe[0]); let mut reader = BufReader::new(file); let mut buffer = String::new(); loop { diff --git a/android-activity/src/native_activity/glue.rs b/android-activity/src/native_activity/glue.rs index d7014d1..003eaa5 100644 --- a/android-activity/src/native_activity/glue.rs +++ b/android-activity/src/native_activity/glue.rs @@ -835,28 +835,31 @@ 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::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC); 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); - } + 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); } } - }); - } + } + }); log::trace!( "Creating: {:p}, saved_state = {:p}, save_state_size = {}",