Close stream when client responds with an entity #45569
Conversation
This shouldn't have an effect given the type of buffers we currently use, but if we change things in the future, this call will be necessary
There was a problem hiding this comment.
I'm not entirely sure if we should do it this way or close the one from the response (or both).
I wonder if it's too early to do that given we don't really know how things will pan out. It's definitely something we will have to think about but I'm not sure we have all the information to do so atm.
I will let you judge of that, though.
|
I want to take it step by step and this one is the safest and most straightforward to do. We probably will at some point need a more clear place to close everything. |
| @@ -358,7 +358,7 @@ public void handle(AsyncResult<Buffer> ar) { | |||
| try { | |||
| if (buffer.length() > 0) { | |||
| requestContext.setResponseEntityStream( | |||
| new ByteBufInputStream(buffer.getByteBuf())); | |||
| new ByteBufInputStream(buffer.getByteBuf(), true)); | |||
There was a problem hiding this comment.
closing an unreleaseable buffer won't have any effect AFAIK, but is fine to do it.
In case the buffer is not releaseable, instead.
The problem arise if the src buffer is used by the caller elsewhere, since we slice it without increasing the ref cnt, when the stream is closed, we release the src as well - which can be dangerous.
I think the problem here is the Vertx API which doesn't help to understand what to do with this...
There was a problem hiding this comment.
This codepath should not result in the buffer being reused anywhere else other than were we use it
I think the problem here is the Vertx API which doesn't help to understand what to do with this...
Agreed
This comment has been minimized.
This comment has been minimized.
Status for workflow
|
This shouldn't have an effect given the type of
buffers we currently use, but if we change things
in the future, this call will be necessary