Repository: netty/netty
Issue: Make SslBufferPool's methods public
Labels: improvement

We should make all Methods of SslBufferPool public so its easy to have a custom implementation in another package. For example a user may want to have a custom impl that use direct ByteBuffers.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/jboss/netty/handler/ssl/DefaultSslBufferPool.java b/src/main/java/org/jboss/netty/handler/ssl/DefaultSslBufferPool.java
new file mode 100644
index 00000000000..3c2dc387bde
--- /dev/null
+++ b/src/main/java/org/jboss/netty/handler/ssl/DefaultSslBufferPool.java
@@ -0,0 +1,109 @@
+/*
+ * Copyright 2011 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package org.jboss.netty.handler.ssl;
+
+import java.nio.ByteBuffer;
+
+import javax.net.ssl.SSLEngine;
+
+public class DefaultSslBufferPool implements SslBufferPool {
+
+    private static final int DEFAULT_POOL_SIZE = MAX_PACKET_SIZE * 1024;
+
+    private final ByteBuffer[] pool;
+    private final int maxBufferCount;
+    private int index;
+
+    /**
+     * Creates a new buffer pool whose size is {@code 18113536}, which can
+     * hold {@code 1024} buffers.
+     */
+    public DefaultSslBufferPool() {
+        this(DEFAULT_POOL_SIZE);
+    }
+
+    /**
+     * Creates a new buffer pool.
+     *
+     * @param maxPoolSize the maximum number of bytes that this pool can hold
+     */
+    public DefaultSslBufferPool(int maxPoolSize) {
+        if (maxPoolSize <= 0) {
+            throw new IllegalArgumentException("maxPoolSize: " + maxPoolSize);
+        }
+
+        int maxBufferCount = maxPoolSize / MAX_PACKET_SIZE;
+        if (maxPoolSize % MAX_PACKET_SIZE != 0) {
+            maxBufferCount ++;
+        }
+
+        pool = new ByteBuffer[maxBufferCount];
+        this.maxBufferCount = maxBufferCount;
+    }
+
+    /**
+     * Returns the maximum size of this pool in byte unit.  The returned value
+     * can be somewhat different from what was specified in the constructor.
+     */
+    public int getMaxPoolSize() {
+        return maxBufferCount * MAX_PACKET_SIZE;
+    }
+
+    /**
+     * Returns the number of bytes which were allocated but have not been
+     * acquired yet.  You can estimate how optimal the specified maximum pool
+     * size is from this value.  If it keeps returning {@code 0}, it means the
+     * pool is getting exhausted.  If it keeps returns a unnecessarily big
+     * value, it means the pool is wasting the heap space.
+     */
+    public synchronized int getUnacquiredPoolSize() {
+        return index * MAX_PACKET_SIZE;
+    }
+
+    public ByteBuffer acquireBuffer() {
+        return acquire();
+    }
+    
+    /**
+     * Will get removed. Please use {@link #acquireBuffer()}
+     * 
+     */
+    @Deprecated
+    synchronized ByteBuffer acquire() {
+        if (index == 0) {
+            return ByteBuffer.allocate(MAX_PACKET_SIZE);
+        } else {
+            return (ByteBuffer) pool[-- index].clear();
+        }
+    }
+    
+    public void releaseBuffer(ByteBuffer buffer) {
+        release(buffer);
+    }
+
+    /**
+     * Will get removed. Please use {@link #releaseBuffer(ByteBuffer)}
+     * 
+     * @deprecated
+     * 
+     */
+    @Deprecated
+    synchronized void release(ByteBuffer buffer) {
+        if (index < maxBufferCount) {
+            pool[index ++] = buffer;
+        }
+    }
+}
diff --git a/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java b/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java
index 05ba7b96a28..642aedaaade 100644
--- a/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java
+++ b/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java
@@ -17,15 +17,10 @@
 
 import java.nio.ByteBuffer;
 
-import javax.net.ssl.SSLEngine;
-
 /**
  * A {@link ByteBuffer} pool dedicated for {@link SslHandler} performance
  * improvement.
- * <p>
- * In most cases, you won't need to create a new pool instance because
- * {@link SslHandler} has a default pool instance internally.
- * <p>
+ *
  * The reason why {@link SslHandler} requires a buffer pool is because the
  * current {@link SSLEngine} implementation always requires a 17KiB buffer for
  * every 'wrap' and 'unwrap' operation.  In most cases, the actual size of the
@@ -33,103 +28,21 @@
  * buffer for every 'wrap' and 'unwrap' operation wastes a lot of memory
  * bandwidth, resulting in the application performance degradation.
  */
-public class SslBufferPool {
-
-    // Add 1024 as a room for compressed data and another 1024 for Apache Harmony compatibility.
-    private static final int MAX_PACKET_SIZE = 16665 + 2048;
-    private static final int DEFAULT_POOL_SIZE = MAX_PACKET_SIZE * 1024;
-
-    private final ByteBuffer[] pool;
-    private final int maxBufferCount;
-    private int index;
-
-    /**
-     * Creates a new buffer pool whose size is {@code 18113536}, which can
-     * hold {@code 1024} buffers.
-     */
-    public SslBufferPool() {
-        this(DEFAULT_POOL_SIZE);
-    }
-
-    /**
-     * Creates a new buffer pool.
-     *
-     * @param maxPoolSize the maximum number of bytes that this pool can hold
-     */
-    public SslBufferPool(int maxPoolSize) {
-        if (maxPoolSize <= 0) {
-            throw new IllegalArgumentException("maxPoolSize: " + maxPoolSize);
-        }
-
-        int maxBufferCount = maxPoolSize / MAX_PACKET_SIZE;
-        if (maxPoolSize % MAX_PACKET_SIZE != 0) {
-            maxBufferCount ++;
-        }
+public interface SslBufferPool {
 
-        pool = new ByteBuffer[maxBufferCount];
-        this.maxBufferCount = maxBufferCount;
-    }
-
-    /**
-     * Returns the maximum size of this pool in byte unit.  The returned value
-     * can be somewhat different from what was specified in the constructor.
-     */
-    public int getMaxPoolSize() {
-        return maxBufferCount * MAX_PACKET_SIZE;
-    }
-
-    /**
-     * Returns the number of bytes which were allocated but have not been
-     * acquired yet.  You can estimate how optimal the specified maximum pool
-     * size is from this value.  If it keeps returning {@code 0}, it means the
-     * pool is getting exhausted.  If it keeps returns a unnecessarily big
-     * value, it means the pool is wasting the heap space.
-     */
-    public synchronized int getUnacquiredPoolSize() {
-        return index * MAX_PACKET_SIZE;
-    }
+    // Returned buffers must be large enough to accomodate the maximum SSL record size.
+    // Header (5) + Data (2^14) + Compression (1024) + Encryption (1024) + MAC (20) + Padding (256)
+    int MAX_PACKET_SIZE = 18713;
 
     /**
      * Acquire a new {@link ByteBuffer} out of the {@link SslBufferPool}
-     * 
      */
-    public ByteBuffer acquireBuffer() {
-        return acquire();
-    }
-    
-    /**
-     * Will get removed. Please use {@link #acquireBuffer()}
-     * 
-     */
-    @Deprecated
-    synchronized ByteBuffer acquire() {
-        if (index == 0) {
-            return ByteBuffer.allocate(MAX_PACKET_SIZE);
-        } else {
-            return (ByteBuffer) pool[-- index].clear();
-        }
-    }
-    
+    ByteBuffer acquireBuffer();
 
     /**
-     * Release a previous acquired {@link ByteBuffer}
-     * 
+     * Release a previously acquired {@link ByteBuffer}
+     *
      * @param buffer
      */
-    public void releaseBuffer(ByteBuffer buffer) {
-        release(buffer);
-    }
-
-    /**
-     * Will get removed. Please use {@link #releaseBuffer(ByteBuffer)}
-     * 
-     * @deprecated
-     * 
-     */
-    @Deprecated
-    synchronized void release(ByteBuffer buffer) {
-        if (index < maxBufferCount) {
-            pool[index ++] = buffer;
-        }
-    }
+    void releaseBuffer(ByteBuffer buffer);
 }
diff --git a/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java b/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java
index e05f21a81f3..d040cd2b84c 100644
--- a/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java
+++ b/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java
@@ -171,7 +171,7 @@ public class SslHandler extends FrameDecoder
      */
     public static synchronized SslBufferPool getDefaultBufferPool() {
         if (defaultBufferPool == null) {
-            defaultBufferPool = new SslBufferPool();
+            defaultBufferPool = new DefaultSslBufferPool();
         }
         return defaultBufferPool;
     }


Test patch: