iffse/pay-respects

[Bug]: Interactive mode freezes on windows

Opened this issue · 11 comments

What happened?

PR gets freeze in interactive mode
Neither Ctrl-C nor Enter is working

The command and output

  1. type 'sshp'
  2. accept to run 'ssh'
  3. ssh failed, pay-respects get stuck

Build information

mokurin000@c789152

No similar issue

  • I have searched the issue list and there is no similar issue found
iffse commented

Relevant line:

.stdout(stderr().as_handle().try_clone_to_owned().unwrap())

Looks like an implementation issue:

This type’s .to_owned() implementation returns another BorrowedHandle rather than an OwnedHandle. It just makes a trivial copy of the raw handle, which is then borrowed under the same lifetime.

Relevant line:

.stdout(stderr().as_handle().try_clone_to_owned().unwrap())

Looks like an implementation issue:

This type’s .to_owned() implementation returns another BorrowedHandle rather than an OwnedHandle. It just makes a trivial copy of the raw handle, which is then borrowed under the same lifetime.

Strange. They said try_clone_to_owned will return an OwnedHandle.
May a bug from the inner implementation

iffse commented

let's see if 536ebeb solves the problem without breaking anything

It seems the issue was not solved
PR feezes at

std::io::stdin().read_line(&mut String::new()).unwrap();

iffse commented

Does other methods freeze too? E.g. replacing stdin with: let _ = Text::new("foo").prompt();

The issue only occurs when I am using msys bash inside Windows Terminal/VSCode Terminal

image

My Windows Terminal profile:

image

With Mintty, MSYS bash works fine

This seems to be a fix

diff --git a/src/suggestions.rs b/src/suggestions.rs
index 10a2b9d..1f65983 100644
--- a/src/suggestions.rs
+++ b/src/suggestions.rs
@@ -349,8 +349,9 @@ fn run_suggestion(shell: &str, command: &str) -> std::process::ExitStatus {
 		.arg("-c")
 		.arg(command)
 		// .stdout(Stdio::inherit())
-		.stdout(stderr())
-		.stderr(Stdio::inherit())
+		.stdout(Stdio::null())
+		.stderr(Stdio::null())
+		.stdin(Stdio::null())
 		.spawn()
 		.expect("failed to execute process")
 		.wait()

just a PoC, doing this will lose all output.

Also you can change all the Stdio::null() into Stdio::piped() which would work fine, but not inherit. It's also needed to explicitly set stdin, which defaults to inherit when calling spawn.

As a result, we may consider manually pipe the subprocess I/O to and from, at least on Windows with bash.

iffse commented

manually pipe the subprocess I/O to and from

That's okay. But can get complicated as you have to pipe stdout to stderr without letting the shell know you are piping to stderr (which strips all color output).

iffse commented

@mokurin000 can you try if appending 1>&2 (redirect stdout to stderr) to the command and the following changes still freezes?

-		.stdout(stderr())
-		.stderr(Stdio::inherit())
+		.stdout(Stdio::null())
+		.stderr(Stdio::piped())
+		.stdin(Stdio::null())
iffse commented

The suggestion was running in another thread (needlessly) so maybe that could be the issue.

Try 4738dd2 when you have time

The suggestion was running in another thread (needlessly) so maybe that could be the issue.

Try 4738dd2 when you have time

I am busy on waylyrics
I will try it soon