2.1.x

[kc7bfi]

There were some places where buffers were not properly duplicated and/or freed. This caused poor performance when attempting to cache IOBuffers.

[elecharny]

Thanks, David !

We are reviewing the patch. Most of the time, it's not doing anything unless you are using cached buffers. I assume this is teh case for you, then the buffer aren't getting cached if they aren't freed.

[kc7bfi]

Any idea when this pull request might be accepted into the mina code base? David R Robison Senior Systems Engineer [cid:image001.png@01D6BD8D.119EB360] From: Emmanuel Lécharny <notifications@github.com> Sent: Thursday, September 24, 2020 10:08 AM To: apache/mina <mina@noreply.github.com> Cc: David Robison <david.robison@psgglobal.net>; Author <author@noreply.github.com> Subject: Re: [apache/mina] 2.1.x (#28) Thanks, David ! We are reviewing the patch. Most of the time, it's not doing anything unless you are using cached buffers. I assume this is teh case for you, then the buffer aren't getting cached if they aren't freed. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#28 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAYWJXCYW66KNNT5DWVEA3DSHNHENANCNFSM4RYK2BLA>.

[elecharny]

Patch finally applied....

Sorry for the 4 years delay, lots of day job/family things got in the path...

[mhitchens]

There's a regression in the patch that was pushed separately by @elecharny.

88cb553#diff-450772ac8848cc0e6cc29eef8dec7d3d2b5633a3f6f2c9f676169b3f73f28f6aR242

This changes removeSessionBuffer() to use getAttribute instead of removeAttribute. After this call, the session will still have a reference to the buffer, and if it's a SimpleBuffer, it will not have been freed. This results in doDecode() re-decoding portions of the buffer.

The patch from @kc7bfi is correct but was not merged.

a8385ae#diff-450772ac8848cc0e6cc29eef8dec7d3d2b5633a3f6f2c9f676169b3f73f28f6aR240

[elecharny]

Checking the {{CumulativeProtocolDecoder}} again, the code is under optimal...

Here what I suggest:

diff --git a/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java b/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
index 2750a1b44..b34c2f090 100644
--- a/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
+++ b/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
@@ -142,20 +142,15 @@ public abstract class CumulativeProtocolDecoder extends ProtocolDecoderAdapter {
         // If we have a session buffer, append data to that; otherwise
         // use the buffer read from the network directly.
         if (buf != null) {
-            boolean appended = false;
             // Make sure that the buffer is auto-expanded.
             if (buf.isAutoExpand()) {
                 try {
                     buf.put(in);
-                    appended = true;
+                    buf.flip();
                 } catch (IllegalStateException | IndexOutOfBoundsException e) {
                     // A user called derivation method (e.g. slice()),
                     // which disables auto-expansion of the parent buffer.
                 }
-            }
-
-            if (appended) {
-                buf.flip();
             } else {
                 // Reallocate the buffer if append operation failed due to
                 // derivation or disabled auto-expansion.
@@ -231,18 +226,14 @@ public abstract class CumulativeProtocolDecoder extends ProtocolDecoderAdapter {
      */
     @Override
     public void dispose(IoSession session) throws Exception {
-        IoBuffer oldBuf = (IoBuffer) session.removeAttribute(BUFFER);
-        
-        if (oldBuf != null) {
-            oldBuf.free();
-        }
+        removeSessionBuffer(session);
     }
 
     private void removeSessionBuffer(IoSession session) {
-        IoBuffer oldBuf = (IoBuffer) session.getAttribute(BUFFER);
+        IoBuffer buf = (IoBuffer) session.removeAttribute(BUFFER);
         
-        if (oldBuf != null) {
-            oldBuf.free();
+        if (buf != null) {
+            buf.free();
         }
     }
 
@@ -252,11 +243,7 @@ public abstract class CumulativeProtocolDecoder extends ProtocolDecoderAdapter {
         remainingBuf.order(buf.order());
         remainingBuf.put(buf);
 
-        IoBuffer oldBuf = (IoBuffer) session.removeAttribute(BUFFER);
-        
-        if (oldBuf != null) {
-            oldBuf.free();
-        }
+        removeSessionBuffer(session);
         
         session.setAttribute(BUFFER, remainingBuf);
     }