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

Deno.run supports piped output only up to 65536 bytes #2505

Closed
szhu opened this issue Jun 12, 2019 · 13 comments

Comments

4 participants
@szhu
Copy link

commented Jun 12, 2019

Deno version:

$ deno version
deno: 0.6.0
v8: 7.6.53
typescript: 3.4.1

The following script works:

async function main() {
  let p = Deno.run({
    args: ["python", "-c", 'import sys; sys.stdout.write("*" * 65536)'],
    stdout: "piped",
  })
  await p.status()
  console.log("Done")
}
main()

The following script doesn't:

async function main() {
  let p = Deno.run({
    args: ["python", "-c", 'import sys; sys.stdout.write("*" * 65537)'],
    stdout: "piped",
  })
  await p.status()
  console.log("Done")
}
main()
@mtharrison

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

I think what's happening is the output is being buffered up to a max of 64kb and then blocking until some of it is read, so the process is just hanging and does not exit.

I notice it succeeds if you read some/all output before awaiting the status.

Reading all

async function main() {

  let p = Deno.run({
    args: ["python", "-c", 'import sys; sys.stdout.write("*" * 65537)'],
    stdout: "piped",
  });

  const rawOutput = await p.output();
  console.log(rawOutput.length);

  await p.status();
  console.log("Done");
}

main();

// 65537
// Done

Reading some (and thus making some space in buffer)

async function main() {

  let p = Deno.run({
    args: ["python", "-c", 'import sys; sys.stdout.write("*" * 65537)'],
    stdout: "piped",
  });

  const buff = new Uint8Array(10);
  await p.stdout.read(buff);
  console.log(new TextDecoder().decode(buff));

  await p.status();
  console.log("Done");
}

main();

// **********
// Done

You will see the same in Rust (playground) if you try to wait for the status without reading some of the output

#![allow(unused)]

use std::process::{Command, Stdio};

fn main() {
    
    let mut child = Command::new("python");
    
    child.args(&["-c", "import sys; sys.stdout.write(\"*\" * 65537)"]);
    child.stdout(Stdio::piped());
        
    println!("Child status was {}", child.status().unwrap());
}

Also see related: rust-lang/rust#27152

In summary I don't think this is a bug but perhaps the manual example should be updated as the toy cat program is broken for files > 64kb.

@ry

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

Thanks - this is a very good example - and clearly a bug. I will investigate soon.

@ry ry added the bug label Jun 12, 2019

@mtharrison

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Here's an analogous example in Node, the buffer though is 24kb for Node before no completion events happen without reading some from the pipe:

const ChildProcess = require('child_process');

const child = ChildProcess.spawn('python', ['-c', `import sys; sys.stdout.write("*" * 24577)`], {
	stdio: 'pipe'
});

child.on('exit', (code) => console.log(code));    // won't be called

I'm curious that you think it's a bug though.

@ry

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

Actually after discussing with @piscisaureus, I think it's not a bug.

I was mistaken before - Python isn't exiting, so the status promise shouldn't resolve.

Maybe we need some functionality of dumping stdout to null for subprocesses.

@ry ry removed the bug label Jun 12, 2019

@bartlomieju

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Ref: #2013

@szhu

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Python isn't exiting, so the status promise shouldn't resolve.

It makes sense that the status() promise shouldn't resolve because Python isn't exiting. What doesn't make sense is that Python is called in a way such that it doesn't exit.

I'm curious that you think it's a bug though.

Why do I think it's a bug? Because having a promise never resolve is a important caveat, and this caveat is not mentioned at all in the documentation.


For context, here's (a simplified version of) my use case. I'm trying to make a command that captures the output of git ls-remote. For repos on GitHub with a lot of branches or PRs, this output exceeds 64kb. I had to spent some time debugging why some commands in my tool would mysteriously hang even though no CPU was being used.

@ry

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

Python is blocked on trying to write to stdout, that's why it doesn't exit.

You have to read some of that input first.

@szhu

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

That makes sense -- in Node, I would read stdout chunk by chunk. It looks like in Deno, I can do the same with process.stdout.read(bytes).

In that case, I'm confused about the purpose of process.output(). What is its intended use case?

@ry

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

process.output() buffers the output of a subprocess and returns it to you all at once.

@bartlomieju

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

In that case, I'm confused about the purpose of process.output(). What is its intended use case?

It reads everything from process' stdout- so it won't resolve until there is no more data to be read. Once it resolves you get complete output of subprocess.

@szhu

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

I don't understand how both of these could be true:

You have to read some of that input first.

returns it to you all at once.

Do I need to read the partial output, or do I get all the output at once?

It might help if someone could give a demo of how to run sample the Python program above (or any program) and capture its entire output in a string.

btw, I'm aware that I might not be understanding something that's fairly obvious to the rest of you -- appreciating the quick responses and explanations in this thread.

@ry

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

Read partial:

let buf = new Uint8Array(1);
await process.stdout.read(buf)

Read complete:

let buf = await process.output();
@szhu

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Okay, it finally makes sense now. Thanks everyone for explaining!


Some examples to help out other people in a similar situation:

This will hang forever:

async function main() {
  let p = Deno.run({
    args: ["python", "-c", 'import sys; sys.stdout.write("*" * 65537)'],
    stdout: "piped",
  })
  await p.status()
  doSomethingWith(await p.output())
  console.log("Done")
}
main()

But this works:

async function main() {
  let p = Deno.run({
    args: ["python", "-c", 'import sys; sys.stdout.write("*" * 65537)'],
    stdout: "piped",
  })
  let output = await p.output()
  await p.status()
  doSomethingWith(output)
  console.log("Done")
}
main()

@szhu szhu closed this Jun 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.