Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tail: fix issue #6543 (--pid when reading from stdin) #6582

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
addf7b5
Added in logic in tail to be able to follow stdin when a PID is speci…
Jul 18, 2024
03b3bf1
Add test to follow file until specified PID dies. Remove comment
Jul 19, 2024
05262b6
Merge branch 'uutils:main' into main
just-an-engineer Jul 19, 2024
aa8f2af
Improve pid test for tail
Jul 19, 2024
bbefeba
cargo fmt
Jul 20, 2024
cbbef09
Increase assert is alive in test_following_with_pid in test_tail to 1…
Jul 23, 2024
520e8b8
Added in logic in tail to be able to follow stdin when a PID is speci…
Jul 18, 2024
b242461
Add test to follow file until specified PID dies. Remove comment
Jul 19, 2024
465ef45
Improve pid test for tail
Jul 19, 2024
731e4e1
cargo fmt
Jul 20, 2024
fed2705
Increase assert is alive in test_following_with_pid in test_tail to 1…
Jul 23, 2024
a4cc3a7
Changed test_following_with_pid to read from a file instead of stdio:…
Jul 28, 2024
e7d1c0b
Merge branch 'main' of https://github.com/just-an-engineer/coreutils …
Jul 28, 2024
1ade9ac
Merge branch 'main' into main
just-an-engineer Jul 28, 2024
0811fea
Apple doesn't currently work for passing in files to stdin
Jul 29, 2024
73bbd9a
Merge branch 'main' of https://github.com/just-an-engineer/coreutils …
Jul 29, 2024
bcc144b
Got test_following_with_pid to work on Windows
just-an-engineer Jul 29, 2024
b8c5bc8
is_not_alive -> is_alive in test_following_with_pid
Jul 29, 2024
b120a56
Fixed `is_not_alive` issue on linux by reaping child sleep process, o…
Jul 29, 2024
8c5097a
Add missed `mut` to windows sleep_command in test_tail for test_follo…
just-an-engineer Jul 29, 2024
1f0d9d5
Fixed formatting, spelling, and made clippy happy
Jul 30, 2024
98c546f
Merge branch 'main' into main
sylvestre Jul 30, 2024
09e2610
Merge branch 'main' into main
just-an-engineer Aug 7, 2024
59aea7e
Merge branch 'main' into main
sylvestre Sep 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/uu/tail/src/tail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,9 @@ fn uu_tail(settings: &Settings) -> UResult<()> {
// Add `path` and `reader` to `files` map if `--follow` is selected.
for input in &settings.inputs.clone() {
match input.kind() {
// File points to /dev/stdin here
InputKind::Stdin => {
tail_stdin(settings, &mut printer, input, &mut observer)?;
}
// and here. Currently, on Linux, it seems that the presence and absence of this
// makes no difference, but leaving this here for verbosity and correctness
InputKind::File(path) if cfg!(unix) && path == &PathBuf::from(text::DEV_STDIN) => {
tail_stdin(settings, &mut printer, input, &mut observer)?;
}
Expand Down
37 changes: 37 additions & 0 deletions tests/by-util/test_tail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4824,3 +4824,40 @@ fn test_obsolete_encoding_windows() {
.stderr_is("tail: bad argument encoding: '-�b'\n")
.code_is(1);
}

#[test]
fn test_following_with_pid() {
use std::process::Command;
use std::time::Duration;
use std::thread::sleep;

let scene = TestScenario::new(util_name!());

let sleep_command = Command::new("sleep")
.arg("10")
BenWiederhake marked this conversation as resolved.
Show resolved Hide resolved
.spawn()
.expect("failed to start sleep command");

let sleep_pid = sleep_command.id();

// when -f is specified, tail should die after
// the pid from --pid also dies
let mut child = scene
.ucmd()
.args(&["--pid", &sleep_pid.to_string(), "-f"])
.set_stdin(Stdio::null())
.stderr_to_stdout()
.run_no_wait();

sleep(Duration::from_secs(1));

child.make_assertion_with_delay(500).is_alive();
BenWiederhake marked this conversation as resolved.
Show resolved Hide resolved

Command::new("kill")
.arg("-9")
.arg(sleep_pid.to_string())
.output()
.expect("failed to kill sleep command");

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After killing sleep, we have a perfect opportunity to quickly check that tail really does terminate by itself. Maybe insert something like this?

    child.make_assertion_with_delay(1000).is_alive();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, for checking if it's is_alive, why not is_not_alive. Running the commands manually, I confirm that the tail exits after sleep exits, and after it's killed. But, I changed it to is_not_alive, and it failed on Linux. Although it passed on Windows. I need to switch back over to Windows and confirm it's breaking now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer my previous question, I believe it should be is_not_alive, and the cause for failure was because sleep's parent was the Rust test program, and without exiting, and without reaping, it became a zombie. However, the libc kill function in is_dead saw it as still alive (returning 0), and the reason the program exited before was because of recv_timeout(settings.sleep_sec) in watch.rs (currently line 532 in this commit), which I assume errored out, leading to the program ending, and leading to the appearance that it detected the pid specified dying and following suite, when it wasn't actually the case.

child.kill();
}