Reworked buffer releasing to ensure that it is always executed before

fillInterested() is called.
This is needed to avoid race conditions where fillInterested()
triggers a new thread entering onFillable() and acquiring a new buffer
while the previous thread is releasing the previous buffer.

1 files changed, 37 insertions, 30 deletions

diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java
index 3b90ddc4c1..e0575bf1fa 100644
--- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java
+++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java
@@ -100,61 +100,68 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec
     @Override
     public void onFillable()
     {
+        buffer = acquireBuffer();
+        process(buffer);
+    }
+
+    private ByteBuffer acquireBuffer()
+    {
         HttpClient client = destination.getHttpClient();
         ByteBufferPool bufferPool = client.getByteBufferPool();
-        buffer = bufferPool.acquire(client.getResponseBufferSize(), true);
-        process();
+        return bufferPool.acquire(client.getResponseBufferSize(), true);
     }
 
-    private void process()
+    private void releaseBuffer(ByteBuffer buffer)
     {
-        if (readAndParse())
-        {
-            HttpClient client = destination.getHttpClient();
-            ByteBufferPool bufferPool = client.getByteBufferPool();
-            bufferPool.release(buffer);
-            // Don't linger the buffer around if we are idle.
-            buffer = null;
-        }
+        assert this.buffer == buffer;
+        HttpClient client = destination.getHttpClient();
+        ByteBufferPool bufferPool = client.getByteBufferPool();
+        bufferPool.release(buffer);
+        this.buffer = null;
     }
 
-    private boolean readAndParse()
+    private void process(ByteBuffer buffer)
     {
-        EndPoint endPoint = getEndPoint();
-        ByteBuffer buffer = this.buffer;
-        while (true)
+        try
         {
-            try
+            EndPoint endPoint = getEndPoint();
+            boolean looping = false;
+            while (true)
             {
-                if (parse(buffer))
-                    return false;
+                if (!looping && parse(buffer))
+                    return;
 
                 int read = endPoint.fill(buffer);
-                if (LOG.isDebugEnabled()) // Avoid boxing of variable 'read'.
+                if (LOG.isDebugEnabled())
                     LOG.debug("Read {} bytes from {}", read, endPoint);
+
                 if (read > 0)
                 {
                     if (parse(buffer))
-                        return false;
+                        return;
                 }
                 else if (read == 0)
                 {
+                    releaseBuffer(buffer);
                     fillInterested();
-                    return true;
+                    return;
                 }
                 else
                 {
+                    releaseBuffer(buffer);
                     shutdown();
-                    return true;
+                    return;
                 }
+
+                looping = true;
             }
-            catch (Exception x)
-            {
-                if (LOG.isDebugEnabled())
-                    LOG.debug(x);
-                close(x);
-                return false;
-            }
+        }
+        catch (Exception x)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug(x);
+            releaseBuffer(buffer);
+            close(x);
         }
     }
 
@@ -352,7 +359,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec
                             {
                                 if (LOG.isDebugEnabled())
                                     LOG.debug("Content consumed asynchronously, resuming processing");
-                                process();
+                                process(HttpConnectionOverFCGI.this.buffer);
                             }
 
                             @Override