-
Notifications
You must be signed in to change notification settings - Fork 84
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
stdout/err buffer overflow behaviour #77
Comments
I had previously thought that NuProcess handled back-pressure. As per brettwooldridge/NuProcess#77, it doesn't appear so. Hence, I'm reverting the code to use a Source.queue and managing a 64KiB buffer directly. If NuProcess ever does handle back-pressure then the PublishIfAvailable source could make a come-back. It was working beautifully. :-)
I had previously thought that NuProcess handled back-pressure. As per brettwooldridge/NuProcess#77, it doesn't appear so. Hence, I'm reverting the code to use a Source.queue and managing a 64KiB buffer directly. If NuProcess ever does handle back-pressure then the PublishIfAvailable source could make a come-back. It was working beautifully. :-)
I had previously thought that NuProcess handled back-pressure. As per brettwooldridge/NuProcess#77, it doesn't appear so. Hence, I'm reverting the code to use a Source.queue and managing a 64KiB buffer directly. If NuProcess ever does handle back-pressure then the PublishIfAvailable source could make a come-back. It was working beautifully. :-)
@huntc Currently, the handler must consume all data. There is no back pressure. There is work to implement back pressure on the |
I can see how one might read that into the The problem with "would then be called at a later time" is: When would that "later time" be, and how would NuProcess know when it is? Currently it's listening for available output, and when there is some it calls back. If the handler doesn't consume that output, though, and the buffer fills up, then NuProcess:
NuProcess would need to grow something that's similar to the existing The This may be something I, or some colleagues on Bitbucket Server, end up circling back to, though, because eventually we'd very much like to have some backpressure in NuProcess. Currently we implement it "around" NuProcess by using the |
I've noticed that for Posix, if the stdout buffer becomes full then a
RuntimeException("stdout buffer has no bytes remaining")
is thrown (https://github.com/brettwooldridge/NuProcess/blob/master/src/main/java/com/zaxxer/nuprocess/internal/BasePosixProcess.java#L483). I've observed that this exception is caught at https://github.com/brettwooldridge/NuProcess/blob/master/src/main/java/com/zaxxer/nuprocess/internal/BaseEventProcessor.java#L83 whereupon the process is then declared as stopped.My understanding from the API doc is that it is fine for a stdout handler to return the buffer that it was passed in the event that it is not in a position to consume it and that the stdout handler would then be called at a later time. My expectation then was that the upstream process would block until it could write more to stdout.
I was hoping that you could confirm the actual design. It seems that stdout handlers must always consume in order to avoid seeing that the process is represented as one that has stopped.
The same behaviour applies to stderr. Thanks.
Note that #53 is related, although that focuses more on a solution. This issue pertains to unexpected behaviour in accordance with the existing API doc.
The text was updated successfully, but these errors were encountered: