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

Update scaladoc for Process #3381

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

daddykotex
Copy link
Contributor

@daddykotex daddykotex commented Jan 22, 2024

I fell into this trap last week. Essentially, I was running a program that looks like this:

//> using scala "3.3.0"
//> using lib "co.fs2::fs2-io:3.9.3"

import cats.effect._
import fs2._

object Main extends IOApp.Simple {
  val run = fs2.io.process
    .ProcessBuilder(
      "git",
      "show",
      "46d8140:lightweight-dynamo.json" // just a large file that exists in the git repo
    )
    .spawn[IO]
    .use { p =>
      p.exitValue *> p.stdout.through(fs2.text.utf8.decode).compile.string
    }
    .flatMap(IO.println)
}

Unfortunately for me, when the file in question (lightweight-dynamo.json in this instance) was somewhat large, my program would hang. I tracked this down to: you must drain stdout if the process you starts is writing to it. Failure to do it can cause the process to block because it's write buffer is full (because nobody reads from it). Relevant java docs.

This change in the documentation should help avoiding such issue. But I think the best thing would be to have a mechanism in place to ensure stdin is drained. I'm not sure how I would implemented this though.

Comment on lines 43 to 44
* and interrupting or otherwise canceling a read-in-progress may kill the process. Not draining
* this `Stream` can block the process and may cause the process to block, or even deadlock.
Copy link
Member

Choose a reason for hiding this comment

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

Same is true of stderr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that but was not sure because the docs on the java side only mention stdin and stdout:

Because some native platforms only provide limited buffer size for standard input and output streams, failure to promptly write the input stream or read the output stream of the subprocess may cause the subprocess to block, or even deadlock.

Copy link
Member

Choose a reason for hiding this comment

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

Oh. Maybe I'm wrong actually and stderr is special. Thanks for pointing it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure it's special, so I've added the warning to stderr as well.

Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

thank you!

@armanbilge armanbilge merged commit 49610a8 into typelevel:main Jan 23, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants