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

Broken pipe Exception from Jersey layer while closing response in ServerRuntime$Responder.writeResponse() not handled or re-thrown #5783

Closed
logovind opened this issue Oct 29, 2024 · 7 comments

Comments

@logovind
Copy link

Exception from Jersey layer while closing response is not handled and implications can very depending on the implementation of ResponseWriter.

For Eg. Helidon is using a custom implementation of the container response writer(JAXRSResponseWriter) which is using a CountdownLatch that is initialized with the value of 1 and decremented to 0 on commit and failure methods. If these methods do not get called then await will hang.

we saw this behavior of await hanging when Broken pipe Exception occurred while closing the response in Jersey code. More details available in helidon-io/helidon#9442

In Jersey, the below code segment in ServerRuntime.writeResponse() method does not handle or re-throw the exception while closing the response. It simply logs the messages and ignores the exception.

https://github.com/eclipse-ee4j/jersey/blob/3.1.9/core-server/src/main/java/org/glassfish/jersey/server/ServerRuntime.java#L467

ServerRuntime.writeResponse()
 
if (close) {
                        try {
                            response.close();
                        } catch (final Exception e) {
                            LOGGER.log(Level.SEVERE, LocalizationMessages.ERROR_CLOSING_COMMIT_OUTPUT_STREAM(), e);
                        }
                    }
 

For successful cases, the implementation of the close calls the “getResponseWriter().commit()” which would have decrement the countDownLatch to 0 and make sure that await() would not block.

https://github.com/eclipse-ee4j/jersey/blob/3.1.9/core-server/src/main/java/org/glassfish/jersey/server/ContainerResponse.java#L404

ContainerResponse.close()
public void close() {
        if (!closed) {
            closed = true;
            messageContext.close();
            requestContext.getResponseWriter().commit();
            requestContext.setWorkers(null);
        }
    }

However, for the error cases, failure() method never gets called and causes the await() to block in the case of Helidon use case. The broken pipe exception is thrown from "messageContext.close()".

stack below:

java.io.UncheckedIOException: java.net.SocketException: Broken pipe
  at io.helidon.common.buffers.GrowingBufferData.writeTo(GrowingBufferData.java:69)
  at io.helidon.common.socket.PlainSocket.write(PlainSocket.java:136)
  at io.helidon.common.socket.SocketWriter.writeNow(SocketWriter.java:81)
  at io.helidon.common.socket.SocketWriterDirect.write(SocketWriterDirect.java:43)
  at io.helidon.webserver.http1.Http1ServerResponse$BlockingOutputStream.write(Http1ServerResponse.java:631)
  at io.helidon.webserver.http1.Http1ServerResponse$BlockingOutputStream.write(Http1ServerResponse.java:505)
  at java.base/java.io.BufferedOutputStream.flushBuffer(BufferedOutputStream.java:125)
  at java.base/java.io.BufferedOutputStream.implFlush(BufferedOutputStream.java:252)
  at java.base/java.io.BufferedOutputStream.flush(BufferedOutputStream.java:240)
  at java.base/java.io.FilterOutputStream.close(FilterOutputStream.java:184)
  at io.helidon.webserver.http1.Http1ServerResponse$ClosingBufferedOutputStream.close(Http1ServerResponse.java:801)
  at org.glassfish.jersey.message.internal.CommittingOutputStream.close(CommittingOutputStream.java:251)
  at org.glassfish.jersey.message.internal.OutboundMessageContext.close(OutboundMessageContext.java:568)
  at org.glassfish.jersey.server.ContainerResponse.close(ContainerResponse.java:403)
  at org.glassfish.jersey.server.ServerRuntime$Responder.writeResponse(ServerRuntime.java:763)
  at org.glassfish.jersey.server.ServerRuntime$Responder.processResponse(ServerRuntime.java:398)
  at org.glassfish.jersey.server.ServerRuntime$Responder.process(ServerRuntime.java:388)
  at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:277)
  -----
  -----
  at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:266)
  at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:253)
  at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:696)
  at io.helidon.microprofile.server.JaxRsService.doHandle(JaxRsService.java:234)
  at io.helidon.microprofile.server.JaxRsService.lambda$handle$2(JaxRsService.java:185)
  at io.helidon.common.context.Contexts.runInContext(Contexts.java:117)
  at io.helidon.microprofile.server.JaxRsService.handle(JaxRsService.java:185)
  at io.helidon.webserver.http.HttpRoutingImpl$RoutingExecutor.doRoute(HttpRoutingImpl.java:169)

we tried out a prototype fix where we modified the below code in Jersey to invoke “request.getResponseWriter().failure(e) “ in the exception block as below.

ServerRuntime.writeResponse() with exception handling
 
if (close) {
                   try {
                            response.close();
                        } catch (final Exception e) {
                            LOGGER.log(Level.WARNING, LocalizationMessages.ERROR_CLOSING_COMMIT_OUTPUT_STREAM(), e);
                            try {
                                request.getResponseWriter().failure(e);
                            } catch (Exception ex) {
                            LOGGER.log(Level.WARNING, LocalizationMessages.ERROR_CLOSING_COMMIT_OUTPUT_STREAM(), e);
                           }
                          }
}

with this change the call to failure decrements the latch and does not cause await to hang in the Helidon implementation of the container response writer.

Would like to check the feasibility of this exception handling change implemented in Jersey.

@jbescos
Copy link
Member

jbescos commented Oct 29, 2024

Do you have a reproducer?. There is some work in progress here:
#5773

In that PR, if the response was already committed but there is IOException (for example, broken pipe), it throws the exception up in the same way it would happen if the response was not committed.

@logovind
Copy link
Author

logovind commented Oct 29, 2024 via email

@vasanth-bhat
Copy link

The change in 5773 how does it take care of invoking "request.getResponseWriter().failure(e);" For Helidon usecase it's critical that thsi get invoked to have the CountDown latch decremented and make sure that thread doesn't hang in await()

@jbescos
Copy link
Member

jbescos commented Oct 30, 2024

request.getResponseWriter().failure(e); gets invoked in #process if there is one exception.

Previously to #5773 , if the response was committed it didn't reach that part. Now it will do it with the following execution stack:

#writeResponse -> this throws now one exception even when the response was committed
#processResponse
#process

@jbescos
Copy link
Member

jbescos commented Oct 30, 2024

@logovind what Jersey version do you need?. I assume it is 3.1.9. I attach it here.

Remove the last .zip and place it and rename it in the way Helidon will use it in classpath.

jersey-server-3.1.99-SNAPSHOT.jar.zip

@logovind
Copy link
Author

logovind commented Nov 6, 2024

I verified the patch from #5786.

This fix working fine independently, even without the Helidon fix from - helidon-io/helidon#9460

In the Jersey fix - #5786, we see that the code changes in the “ContainerResponse.java” where that “requestContext.getResponseWriter().failure(e)” is invoked from the catch block helps to resolve the issue. We confirmed this based on below stack:

java.lang.Exception: Stack trace
at java.base/java.lang.Thread.dumpStack(Thread.java:2209)
at io.helidon.microprofile.server.JaxRsService$JaxRsResponseWriter.failure(JaxRsService.java:386)
at org.glassfish.jersey.server.ContainerResponse.close(ContainerResponse.java:411)
at org.glassfish.jersey.server.ServerRuntime$Responder.writeResponse(ServerRuntime.java:763)
at org.glassfish.jersey.server.ServerRuntime$Responder.processResponse(ServerRuntime.java:398)
at org.glassfish.jersey.server.ServerRuntime$Responder.process(ServerRuntime.java:388)
at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:277)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244)
at org.glassfish.jersey.internal.Errors.process(Errors.java:292)
at org.glassfish.jersey.internal.Errors.process(Errors.java:274)
at org.glassfish.jersey.internal.Errors.process(Errors.java:244)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:266)
at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:253)
at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:696)
at io.helidon.microprofile.server.JaxRsService.doHandle(JaxRsService.java:228)
…..
…..

jbescos added a commit that referenced this issue Nov 12, 2024
…verRuntime.writeResponse() not handled or re-thrown #5783 (#5786)

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos
Copy link
Member

jbescos commented Nov 12, 2024

Merged

@jbescos jbescos closed this as completed Nov 12, 2024
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

No branches or pull requests

3 participants