You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/04/08 00:47:56 UTC

svn commit: r645722 - /tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java

Author: markt
Date: Mon Apr  7 15:47:54 2008
New Revision: 645722

URL: http://svn.apache.org/viewvc?rev=645722&view=rev
Log:
All of the issues I was seeing with mark/reset were due to states that resulted in a call to CharChunk.flushBuffer().
I tried many different ways to fix it but by far the simplest was this patch that just increases the size of the CharChunk internal buffer when creating the mark sufficiently that flushBuffer() is never called.
With this patch I can't break mark/reset with bug 44494's test case in single or multibyte mode.

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java?rev=645722&r1=645721&r2=645722&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java Mon Apr  7 15:47:54 2008
@@ -457,11 +457,7 @@
                 cb.setOffset(0);
             }
         }
-        int offset = readAheadLimit;
-        if (offset < size) {
-            offset = size;
-        }
-        cb.setLimit(cb.getStart() + offset);
+        cb.setLimit(cb.getStart() + readAheadLimit + size);
         markPos = cb.getStart();
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r645722 - /tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
ignore my prev email about the mem leak, I wasn't up to current in 
trunk, and was running into something else

Filip

markt@apache.org wrote:
> Author: markt
> Date: Mon Apr  7 15:47:54 2008
> New Revision: 645722
>
> URL: http://svn.apache.org/viewvc?rev=645722&view=rev
> Log:
> All of the issues I was seeing with mark/reset were due to states that resulted in a call to CharChunk.flushBuffer().
> I tried many different ways to fix it but by far the simplest was this patch that just increases the size of the CharChunk internal buffer when creating the mark sufficiently that flushBuffer() is never called.
> With this patch I can't break mark/reset with bug 44494's test case in single or multibyte mode.
>
> Modified:
>     tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java?rev=645722&r1=645721&r2=645722&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java Mon Apr  7 15:47:54 2008
> @@ -457,11 +457,7 @@
>                  cb.setOffset(0);
>              }
>          }
> -        int offset = readAheadLimit;
> -        if (offset < size) {
> -            offset = size;
> -        }
> -        cb.setLimit(cb.getStart() + offset);
> +        cb.setLimit(cb.getStart() + readAheadLimit + size);
>          markPos = cb.getStart();
>      }
>  
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
>   


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r645722 - /tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
just taking a quick look at the patch, this is a memory leak.
the buffer will simply grow larger and larger until an OOM happens, if 
one keeps marking the buffer
when I am more awake, I'll help out on this, it's very easy to reproduce
http://people.apache.org/~fhanik/upload-mark.jsp

Filip

markt@apache.org wrote:
> Author: markt
> Date: Mon Apr  7 15:47:54 2008
> New Revision: 645722
>
> URL: http://svn.apache.org/viewvc?rev=645722&view=rev
> Log:
> All of the issues I was seeing with mark/reset were due to states that resulted in a call to CharChunk.flushBuffer().
> I tried many different ways to fix it but by far the simplest was this patch that just increases the size of the CharChunk internal buffer when creating the mark sufficiently that flushBuffer() is never called.
> With this patch I can't break mark/reset with bug 44494's test case in single or multibyte mode.
>
> Modified:
>     tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java?rev=645722&r1=645721&r2=645722&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java Mon Apr  7 15:47:54 2008
> @@ -457,11 +457,7 @@
>                  cb.setOffset(0);
>              }
>          }
> -        int offset = readAheadLimit;
> -        if (offset < size) {
> -            offset = size;
> -        }
> -        cb.setLimit(cb.getStart() + offset);
> +        cb.setLimit(cb.getStart() + readAheadLimit + size);
>          markPos = cb.getStart();
>      }
>  
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
>   


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r645722 - /tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java

Posted by Mark Thomas <ma...@apache.org>.
Remy Maucherat wrote:
> On Mon, 2008-04-07 at 22:47 +0000, markt@apache.org wrote:
>> Author: markt
>> Date: Mon Apr  7 15:47:54 2008
>> New Revision: 645722
>>
>> URL: http://svn.apache.org/viewvc?rev=645722&view=rev
>> Log:
>> All of the issues I was seeing with mark/reset were due to states that
>> resulted in a call to CharChunk.flushBuffer().
>> I tried many different ways to fix it but by far the simplest was this
>> patch that just increases the size of the CharChunk internal buffer
>> when creating the mark sufficiently that flushBuffer() is never
>> called.
>> With this patch I can't break mark/reset with bug 44494's test case in
>> single or multibyte mode.
> 
> Oops, I had forgotten that part, there was indeed another problem with
> that. Sorry.

No problem. I learned quite a bit digging around in that code ;)

If you can see a better way of fixing this then feel free to propose an 
alternative patch.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r645722 - /tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java

Posted by Remy Maucherat <re...@apache.org>.
On Mon, 2008-04-07 at 22:47 +0000, markt@apache.org wrote:
> Author: markt
> Date: Mon Apr  7 15:47:54 2008
> New Revision: 645722
> 
> URL: http://svn.apache.org/viewvc?rev=645722&view=rev
> Log:
> All of the issues I was seeing with mark/reset were due to states that
> resulted in a call to CharChunk.flushBuffer().
> I tried many different ways to fix it but by far the simplest was this
> patch that just increases the size of the CharChunk internal buffer
> when creating the mark sufficiently that flushBuffer() is never
> called.
> With this patch I can't break mark/reset with bug 44494's test case in
> single or multibyte mode.

Oops, I had forgotten that part, there was indeed another problem with
that. Sorry.

Rémy



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org