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

4.x: Allow Unbuffered OutputStream for Http1ServerResponse (#9276) #9277

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

VerKWer
Copy link
Contributor

@VerKWer VerKWer commented Sep 23, 2024

Description

Currently, when building a new WebServer instance, one can specify writeBufferSize(0). The server builds and starts normally, but any http1 request handler that tries to use ServerResponse.outputStream() will throw an exception. This is obviously not very user friendly, and we should either fail when trying to build/start the server instance or support unbuffered response OutputStreams.

I propose the latter approach, as it was the default before #6509 was merged. An obvious use case for unbuffered response writer is if the user does their own buffer management. For example, I'm currently moving an existing project to Helidon, which uses its own buffer pool to reduce GC pressure. Not allowing a buffer size of 0 would mean that there is no way to avoid the allocation of a new buffer for every request.

Solves issue #9276

Documentation

I added a new abstract class ResponseOutputStream in Http1ServerResponse, which serves as a base for the two existing classes BlockingOutputStream and ClosingBufferedOutputStream. That way, we can choose which one to use after checking the write buffer size.

I also added the WriteBufferTest class to test both the default and zero write buffer case.

Currently, having a write buffer size of 0 always leads to an exception
when an Http1 request handler tries to create an OutputStream.
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 23, 2024
* (non-zero) write buffer.
*/
@Test
void defaultWriteBufferTest() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

Tests that require startup of a server should go to:
webserver/tests/webserver
where you can also use the @ServerTest annotation. See other tests in the module.
If you do not feel like changing the PR, let me know and I will take it over and do the rest.
Thanks for your contribution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying this. I'll gladly move the tests. How would I go about actually running the tests there? They don't seem to be part of the webserver module, and if I try using webserver/tests as a standalone project, compilation fails due to missing dependencies. I'm sure I'm missing something very obvious here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants