Issue #4331 Close Complete

Soft close for Dispatcher release buffer in onWriteComplete Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty · Dec 10, 2019 · 589b71a · 589b71a
1 parent 7612bb5
commit 589b71a
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 54 deletions.
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java
@@ -220,7 +220,7 @@ protected void forward(ServletRequest request, ServletResponse response, Dispatc
                 _contextHandler.handle(_pathInContext, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response);
 
                 if (!baseRequest.getHttpChannelState().isAsync())
-                    commitResponse(response, baseRequest);
+                    baseRequest.getResponse().softClose();
             }
         }
         finally
@@ -242,57 +242,6 @@ public String toString()
         return String.format("Dispatcher@0x%x{%s,%s}", hashCode(), _named, _uri);
     }
 
-    @SuppressWarnings("Duplicates")
-    private void commitResponse(ServletResponse response, Request baseRequest) throws IOException, ServletException
-    {
-        if (baseRequest.getResponse().isWriting())
-        {
-            try
-            {
-                // Try closing Writer first (based on knowledge in Response obj)
-                response.getWriter().close();
-            }
-            catch (IllegalStateException ex)
-            {
-                try
-                {
-                    // Try closing OutputStream as alternate route
-                    // This path is possible due to badly behaving Response wrappers
-                    response.getOutputStream().close();
-                }
-                catch (IllegalStateException ex2)
-                {
-                    ServletException servletException = new ServletException("Unable to commit the response", ex2);
-                    servletException.addSuppressed(ex);
-                    throw servletException;
-                }
-            }
-        }
-        else
-        {
-            try
-            {
-                // Try closing OutputStream first (based on knowledge in Response obj)
-                response.getOutputStream().close();
-            }
-            catch (IllegalStateException ex)
-            {
-                try
-                {
-                    // Try closing Writer as alternate route
-                    // This path is possible due to badly behaving Response wrappers
-                    response.getWriter().close();
-                }
-                catch (IllegalStateException ex2)
-                {
-                    ServletException servletException = new ServletException("Unable to commit the response", ex2);
-                    servletException.addSuppressed(ex);
-                    throw servletException;
-                }
-            }
-        }
-    }
-
     private class ForwardAttributes implements Attributes
     {
         final Attributes _attr;

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -901,7 +901,7 @@ public void sendError(int code, String message)
                 throw new IllegalStateException("Response is " + _outputState);
 
             response.setStatus(code);
-            response.closedBySendError();
+            response.softClose();
 
             request.setAttribute(ErrorHandler.ERROR_CONTEXT, request.getErrorContext());
             request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI());

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
@@ -267,13 +267,15 @@ void onWriteComplete(boolean last, Throwable failure)
 
         boolean wake = false;
         Callback callback = null;
+        boolean release = false;
         synchronized (_channelState)
         {
             if (_state == State.CLOSING || last)
             {
                 _state = State.CLOSED;
                 callback = _closedCallback;
                 _closedCallback = null;
+                release = true;
             }
 
             switch (_apiState)
@@ -316,6 +318,8 @@ void onWriteComplete(boolean last, Throwable failure)
         }
         finally
         {
+            if (release)
+                releaseBuffer();
             if (wake)
                 _channel.execute(_channel); // TODO can we call directly? Why execute?
         }
@@ -461,6 +465,7 @@ public void closed()
                 _state = State.CLOSED;
             }
         }
+        releaseBuffer();
         if (callback != null)
             callback.succeeded();
     }

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
@@ -145,7 +145,7 @@ public void reopen()
         _out.reopen();
     }
 
-    public void closedBySendError()
+    public void softClose()
     {
         setErrorSent(true);
         _out.closedBySendError();