Skip to content

Commit

Permalink
Close logpipe input file descriptor after dup2()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MarijnS95 committed Oct 16, 2023
1 parent a291e37 commit 707a9d3
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 22 deletions.
16 changes: 11 additions & 5 deletions android-activity/src/game_activity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
37 changes: 20 additions & 17 deletions android-activity/src/native_activity/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}",
Expand Down

0 comments on commit 707a9d3

Please sign in to comment.