From 8cbab09527da418ef84a821e9c3f2d5e1672d79a Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@intalio.com>
Date: Thu, 5 Mar 2015 11:05:35 +1100
Subject: [PATCH] 461452 Double release of buffer by HttpReceiverOverHTTP

This commit is just a tidy up of the code to reduce the size of the race causing this problem.  It is not a fix.
---
 .../client/http/HttpReceiverOverHTTP.java     | 54 ++++++++++---------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java
index b45e8fdd0834..fd2ead37c7d5 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java
@@ -64,27 +64,29 @@ protected ByteBuffer getResponseBuffer()
 
     public void receive()
     {
-        buffer = acquireBuffer();
-        process(buffer);
+        if (buffer==null)
+            acquireBuffer();
+        process();
     }
 
-    private ByteBuffer acquireBuffer()
+    private void acquireBuffer()
     {
         HttpClient client = getHttpDestination().getHttpClient();
         ByteBufferPool bufferPool = client.getByteBufferPool();
-        return bufferPool.acquire(client.getResponseBufferSize(), true);
+        buffer = bufferPool.acquire(client.getResponseBufferSize(), true);
     }
 
-    private void releaseBuffer(ByteBuffer buffer)
+    private void releaseBuffer()
     {
-        assert this.buffer == buffer;
+        if (buffer==null)
+            throw new IllegalStateException();
         HttpClient client = getHttpDestination().getHttpClient();
         ByteBufferPool bufferPool = client.getByteBufferPool();
         bufferPool.release(buffer);
-        this.buffer = null;
+        buffer = null;
     }
 
-    private void process(ByteBuffer buffer)
+    private void process()
     {
         try
         {
@@ -97,11 +99,11 @@ private void process(ByteBuffer buffer)
                 {
                     if (LOG.isDebugEnabled())
                         LOG.debug("{} closed", connection);
-                    releaseBuffer(buffer);
+                    releaseBuffer();
                     return;
                 }
 
-                if (!parse(buffer))
+                if (parse())
                     return;
 
                 int read = endPoint.fill(buffer);
@@ -110,18 +112,18 @@ private void process(ByteBuffer buffer)
 
                 if (read > 0)
                 {
-                    if (!parse(buffer))
+                    if (parse())
                         return;
                 }
                 else if (read == 0)
                 {
-                    releaseBuffer(buffer);
+                    releaseBuffer();
                     fillInterested();
                     return;
                 }
                 else
                 {
-                    releaseBuffer(buffer);
+                    releaseBuffer();
                     shutdown();
                     return;
                 }
@@ -131,7 +133,7 @@ else if (read == 0)
         {
             if (LOG.isDebugEnabled())
                 LOG.debug(x);
-            releaseBuffer(buffer);
+            releaseBuffer();
             failAndClose(x);
         }
     }
@@ -140,10 +142,9 @@ else if (read == 0)
      * Parses a HTTP response from the given {@code buffer}.
      *
      * @param buffer the response bytes
-     * @return true to indicate that the parsing may proceed (for example with another response),
-     * false to indicate that the parsing should be interrupted (and will be resumed by another thread).
+     * @return true to indicate that parsing should be interrupted (and will be resumed by another thread).
      */
-    private boolean parse(ByteBuffer buffer)
+    private boolean parse()
     {
         // Must parse even if the buffer is fully consumed, to allow the
         // parser to advance from asynchronous content to response complete.
@@ -151,13 +152,18 @@ private boolean parse(ByteBuffer buffer)
         if (LOG.isDebugEnabled())
             LOG.debug("Parsed {}, remaining {} {}", handle, buffer.remaining(), parser);
 
-        if (!handle)
-            return true;
+        if (handle)
+        {
+            // There are several cases here:
+            // a) the content is being handled asynchronously and resume will eventually call process().  Return false.
+            // b) the content has already been handled asynchronously, resume called process and processing is ongoing.  Return false.
+            // c) the content has already been handled asynchronously, resume called process which completed the response.  Return false.
+            // d) This call to parse called parseNext, which completed the response. Return true
+
+            return !parser.isStart(); // TODO this is insufficient to distinguish the last to cases above
+        }
 
-        // If the parser returns true, we need to differentiate two cases:
-        // A) the response is completed, so the parser is in START state;
-        // B) the content is handled asynchronously, so the parser is in CONTENT state.
-        return parser.isStart();
+        return false;
     }
 
     protected void fillInterested()
@@ -244,7 +250,7 @@ public void resume()
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("Content consumed asynchronously, resuming processing");
-                process(getResponseBuffer());
+                process();
             }
 
             public void abort(Throwable x)