Mark, On 3/9/15 1:23 PM, Mark Thomas wrote: > On 9 March 2015 16:35:57 GMT+00:00, Christopher Schultz > <ch...@christopherschultz.net> wrote: >> Mark, >> >> On 3/9/15 12:32 PM, ma...@apache.org wrote: >>> Author: markt >>> Date: Mon Mar 9 16:32:43 2015 >>> New Revision: 1665297 >>> >>> URL: http://svn.apache.org/r1665297 >>> Log: >>> Follow up to r1665062 >>> Callers assume that output() will fully write the data that is passed >> to it. Ensure that this is the case when the AJP message size is larger >> than the output buffer. >>> >>> Modified: >>> >> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java >>> >>> Modified: >> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java >>> URL: >> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java?rev=1665297&r1=1665296&r2=1665297&view=diff >>> >> ============================================================================== >>> --- >> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java >> (original) >>> +++ >> tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java >> Mon Mar 9 16:32:43 2015 >>> @@ -302,24 +302,29 @@ public class AjpNioProcessor extends Abs >>> ByteBuffer writeBuffer = >>> >> socketWrapper.getSocket().getBufHandler().getWriteBuffer(); >>> >>> - int toWrite = Math.min(length, writeBuffer.remaining()); >>> - writeBuffer.put(src, offset, toWrite); >>> - >>> - writeBuffer.flip(); >>> - >>> - long writeTimeout = att.getWriteTimeout(); >>> - Selector selector = null; >>> - try { >>> - selector = pool.get(); >>> - } catch ( IOException x ) { >>> - //ignore >>> - } >>> - try { >>> - pool.write(writeBuffer, socketWrapper.getSocket(), >> selector, >>> - writeTimeout, true); >>> - }finally { >>> - writeBuffer.clear(); >>> - if ( selector != null ) pool.put(selector); >>> + int left = length; >>> + int written = 0; >>> + while (left > 0) { >>> + int toWrite = Math.min(left, writeBuffer.remaining()); >>> + writeBuffer.put(src, offset, toWrite); >>> + >>> + writeBuffer.flip(); >>> + >>> + long writeTimeout = att.getWriteTimeout(); >>> + Selector selector = null; >>> + try { >>> + selector = pool.get(); >>> + } catch ( IOException x ) { >>> + //ignore >>> + } >>> + try { >>> + written = pool.write(writeBuffer, >> socketWrapper.getSocket(), >>> + selector, writeTimeout, true); >>> + } finally { >>> + writeBuffer.clear(); >>> + if ( selector != null ) pool.put(selector); >>> + } >>> + left -= written; >>> } >>> } >> >> Okay, yes, this was more like what I was expecting to see. I'll give it >> another try. > > Glad to see it is working now. The next jobs are a) documenting the > expected behavior re full vs partial write and b) take a look at trying > to reduce the number of array copies (or equivalent) that the I/O codes > does.
I'm not sure it's really worth it to fix, at least not for Tomcat 7/8. I'd focus on Tomcat 9 where you guys have been doing good work (except for removing BIO! Boo! Hiss! ... yeah, I know why and I agree but I'm still not terribly happy about it). Definitely reducing otherwise useless copying is good, but don't kill yourself over it in stable versions unless there is a specific complaint. There are too many opportunities for breakage. I'm actually shocked that nobody found this problem before. I guess not that many people use mod_jk, or maybe most of them use it with the default packet size. I only found this because of two events that occurred very far apart in time: 1a. I was doing testing on client certificate handling ... years ago. Client certificates tend to grow large and since the whole request headers must fit into a single AJP packet, I had to increase max_packet_size in mod_jk. I did this for my "template" worker, to affect all workers that derive from it. 1b. I changed my "client certificate test" application to use a matching max packet size to 1a. 2. Recent use of GWT had started sending rather large payloads back and forth to our servers. (Yes, I'm also curious why GWT, supposedly to improve performance, is now sending more data to and from the server than just refreshing the page... but where was I?) This notified us that the packetSize in Tomcat was incorrect. Adjusting the packetSize in Tomcat immediately caused all hell to break loose. Well, at any rate, I'm glad it's fixed, now. Thanks, -chris
signature.asc
Description: OpenPGP digital signature
