Deno.run with piped stdout gets stuck in specific scenarios

[lucacasonato]

Reproduction

1. Generate test file:

echo "hello world" > file.txt
for i in {1..10}; do cat file.txt file.txt > file2.txt && mv file2.txt file.txt; done

2. create test.js

const proc = Deno.run({
  cmd: ["cat", "test.json"],
  stdout: "piped",
  stderr: "piped"
});

const status = await proc.status();

3. run test.ts

deno --allow-run test.js

4. Deno gets stuck on await proc.status().

[ry]

I can't repeat it.

[lucacasonato]

Managed to narrow it down to only happening when the file that is being opened is stored on an NTFS drive - this is really weird. It's not happening on macOS for me either, just on Linux + file.txt on NTFS drive. If the file.txt is on ext4 its fine. I'll try on Windows and report back. This might be a weird NTFS driver bug on Linux though - had some of those before.

[lucacasonato]

Checked on windows - not happening there. I'll close this and reopen if I have any more exact information about what is going on.

[lucacasonato]

Ok I have a reliable reproduction, on macOS now:

1. Save this file as lib.deno.d.ts: https://gist.github.com/lucacasonato/9a81ef88fa7f43ba8fc1297a04187ac9

2. Create test.js:

const proc = Deno.run({
  cmd: ["deno", "doc", "lib.deno.d.ts", "--json", "--reload"],
  stdout: "piped",
  stderr: "piped"
});

const status = await proc.status();

3. Run deno --allow-run test.js

4. Deno gets stuck on await proc.status().

[bartlomieju]

@lucacasonato does error happen if you await proc.output() and await proc.stderrOutput() before awaiting on status? It seems that unit_test_runner produces similar sized output and works without a problem.

[lucacasonato]

Everything works as expected when I add await proc.output() between the run call and the proc.status()call. await proc.stderrOutput() has no effect.

[youurayy]

This happens when the stdout is bigger than (?) 64KiB, consider:

const p = Deno.run({

  // this will work in any case:
  // cmd: [ 'curl', '-s', 'https://deno.land' ], // 4KB

  // this will work only if stdout is drained below:
  cmd: [ 'curl', '-s', 'https://en.wikipedia.org/wiki/Deno_(software)' ], // 79KB

  stdout: 'piped',
  stderr: 'piped'
})

console.log('here 1')

// await p.output() // drain the stdout here
console.log('here 1.1')

await p.stderrOutput()
console.log('here 1.2')

const { code } = await p.status()
console.log('here 2, code: ' + code)

You have to uncomment the // drain the stdout here line to make the Wikipedia link work.
Tested on Windows.
Run with deno run --allow-run test.js.

[lucacasonato]

I just ran into this again on the registry. It is a very annoying bug because it is so not logical. I think many people have and will run into it. I also ran into it for doc.deno.land. We should really address this one soon.

[exside]

@lucacasonato I think I did run into some form of it: #7208 (comment) but can't explain it...

[ebebbington]

@lucacasonato I get the same issue when the sub process is asking for stdin (similar to what i mentioned before), it just hangs on the await p.status(), not sure in this scenario if it's possible to address or even a bug

[roman-kirsanov]

encountered same issue again! here is my work around:

const proc = Deno.run({ cmd, stderr: 'piped', stdout: 'piped' });
const [ stderr, stdout, status ] = await Promise.all([ proc.stderrOutput(), proc.output(), proc.status() ]);

[Soremwar]

Still happening on latest canary

[bartlomieju]

So after further investigation I think this works as it should.

stdout and stderr pipes should be closed manually before awaiting on the status, so @rok-star's solution is the preferable one for the time being. I will update the docs for the Deno.run API accordingly (edit: #9390), but overall the API is not very user-friendly and I think it might have to undergo a rework for 2.0 to make it more like Tokio's Child API (I will open a separate issue for it).

@lucacasonato I get the same issue when the sub process is asking for stdin (similar to what i mentioned before), it just hangs on the await p.status(), not sure in this scenario if it's possible to address or even a bug

@ebebbington this should no longer be the case, calling p.status() will close stdin of the child process before awaiting on the status to avoid the deadlock (https://docs.rs/tokio/1.1.1/tokio/process/struct.Child.html#method.wait).

[ebebbington]

@bartlomieju ok cool thanks, I’ll just mention that this issue doesn’t affect me anymore I guess as I moved to another solution but good to hear it is addressed

[dgreensp]

This was very surprising to encounter, and I don't know what I would have done if I hadn't found this issue!

FWIW the examples of Deno.run that I was working off of (incorrectly) read status before output, e.g. in the manual: https://deno.land/manual/examples/subprocess . I wouldn't have thought to specifically read the docs for the Process type.

[aapoalas]

Is this still happening, or has the deadlock been fixed?

[tiesselune]

This is still happening (just ran into it using Deno.run, but the documentation does warn the user that stdout and stderr must be closed when piped so the process can exit. (So output must be read, basically, before calling status)

The surprising part is that it does not happen on "small" outputs, only on big ones, which is a bit destabilizing (and can result in seemingly arbitrary hangs when other commands succeed), reason for which I stumbled on this thread in the first place...

[crowlKats]

This doesn't happen with the new subprocess API (Deno.Command), so closing since Deno.run is deprecated.