Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix #6562 last written bytebuffer (#6563) (#6579) 

Fixes #6562 the last written bytebuffer calculation.
Also fixed an associated issue with unnecessary flush of an empty when last calculation already signalled last.
This commit is contained in:
Greg Wilkins 2021-08-05 09:13:12 +10:00 committed by GitHub
parent 266d8f0dca
commit b0140dae05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 199 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
View File

@ -847,10 +847,12 @@ public class HttpOutput extends ServletOutputStream implements Runnable
// Blocking write
try
{
boolean complete = false;
// flush any content from the aggregate
if (BufferUtil.hasContent(_aggregate))
{
channelWrite(_aggregate, last && len == 0);
complete = last && len == 0;
channelWrite(_aggregate, complete);
// should we fill aggregate again from the buffer?
if (len > 0 && !last && len <= _commitSize && len <= maximizeAggregateSpace())
@ -880,7 +882,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable
}
channelWrite(view, last);
}
else if (last)
else if (last && !complete)
{
channelWrite(BufferUtil.EMPTY_BUFFER, true);
}
@ -907,7 +909,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable
{
checkWritable();
long written = _written + len;
last = _channel.getResponse().isAllContentWritten(_written);
last = _channel.getResponse().isAllContentWritten(written);
flush = last || len > 0 || BufferUtil.hasContent(_aggregate);
if (last && _state == State.OPEN)
@ -951,13 +953,17 @@ public class HttpOutput extends ServletOutputStream implements Runnable
{
// Blocking write
// flush any content from the aggregate
boolean complete = false;
if (BufferUtil.hasContent(_aggregate))
channelWrite(_aggregate, last && len == 0);
{
complete = last && len == 0;
channelWrite(_aggregate, complete);
}
// write any remaining content in the buffer directly
if (len > 0)
channelWrite(buffer, last);
else if (last)
else if (last && !complete)
channelWrite(BufferUtil.EMPTY_BUFFER, true);
onWriteComplete(last, null);

194
jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java
View File

@ -22,6 +22,8 @@ import java.nio.ByteBuffer;
import java.nio.channels.ReadableByteChannel;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.AsyncContext;
import javax.servlet.ServletException;
@ -36,6 +38,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.HotSwapHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.FuturePromise;
import org.eclipse.jetty.util.resource.Resource;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
@ -45,6 +48,7 @@ import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -355,6 +359,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -369,6 +374,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -383,6 +389,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -397,6 +404,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -414,6 +422,7 @@ public class HttpOutputTest
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -428,6 +437,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -442,6 +452,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -456,6 +467,52 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
public void testWriteBufferSmallKnown() throws Exception
{
final Resource big = Resource.newClassPathResource("simple/big.txt");
_handler._writeLengthIfKnown = true;
_handler._content = BufferUtil.toBuffer(big, false);
_handler._byteBuffer = BufferUtil.allocate(8);
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
public void testWriteBufferMedKnown() throws Exception
{
final Resource big = Resource.newClassPathResource("simple/big.txt");
_handler._writeLengthIfKnown = true;
_handler._content = BufferUtil.toBuffer(big, false);
_handler._byteBuffer = BufferUtil.allocate(4000);
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
public void testWriteBufferLargeKnown() throws Exception
{
final Resource big = Resource.newClassPathResource("simple/big.txt");
_handler._writeLengthIfKnown = true;
_handler._content = BufferUtil.toBuffer(big, false);
_handler._byteBuffer = BufferUtil.allocate(8192);
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -471,6 +528,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -486,6 +544,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -501,6 +560,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -516,12 +576,13 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
public void testAsyncWriteHuge() throws Exception
{
_handler._writeLengthIfKnown = true;
_handler._writeLengthIfKnown = false;
_handler._content = BufferUtil.allocate(4 * 1024 * 1024);
_handler._content.limit(_handler._content.capacity());
for (int i = _handler._content.capacity(); i-- > 0; )
@ -533,7 +594,8 @@ public class HttpOutputTest
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -549,6 +611,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -564,6 +627,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -579,6 +643,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -595,6 +660,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false));
}
@Test
@ -629,6 +695,7 @@ public class HttpOutputTest
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length: 11"));
assertThat(response, containsString("simple text"));
assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true));
}
@Test
@ -686,6 +753,116 @@ public class HttpOutputTest
assertThat(response, containsString("400\tTHIS IS A BIGGER FILE"));
}
@Test
public void testEmptyArray() throws Exception
{
FuturePromise<Boolean> committed = new FuturePromise<>();
AbstractHandler handler = new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
response.setStatus(200);
try
{
response.getOutputStream().write(new byte[0]);
committed.succeeded(response.isCommitted());
}
catch (Throwable t)
{
committed.failed(t);
}
}
};
_swap.setHandler(handler);
handler.start();
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(committed.get(10, TimeUnit.SECONDS), is(false));
}
@Test
public void testEmptyArrayKnown() throws Exception
{
FuturePromise<Boolean> committed = new FuturePromise<>();
AbstractHandler handler = new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
response.setStatus(200);
response.setContentLength(0);
try
{
response.getOutputStream().write(new byte[0]);
committed.succeeded(response.isCommitted());
}
catch (Throwable t)
{
committed.failed(t);
}
}
};
_swap.setHandler(handler);
handler.start();
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length: 0"));
assertThat(committed.get(10, TimeUnit.SECONDS), is(true));
}
@Test
public void testEmptyBuffer() throws Exception
{
AtomicBoolean committed = new AtomicBoolean();
AbstractHandler handler = new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
response.setStatus(200);
((HttpOutput)response.getOutputStream()).write(ByteBuffer.wrap(new byte[0]));
committed.set(response.isCommitted());
}
};
_swap.setHandler(handler);
handler.start();
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(committed.get(), is(false));
}
@Test
public void testEmptyBufferKnown() throws Exception
{
AtomicBoolean committed = new AtomicBoolean();
AbstractHandler handler = new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
response.setStatus(200);
response.setContentLength(0);
((HttpOutput)response.getOutputStream()).write(ByteBuffer.wrap(new byte[0]));
committed.set(response.isCommitted());
}
};
_swap.setHandler(handler);
handler.start();
String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Content-Length: 0"));
assertThat(committed.get(), is(true));
}
@Test
public void testAggregation() throws Exception
{
@ -851,7 +1028,7 @@ public class HttpOutputTest
aggregated += data.length;
}
// write data that will not be aggregated
// write data that will not be aggregated because it is too large
data = new byte[bufferSize + 1];
Arrays.fill(data, (byte)(fill++));
expected.write(data);
@ -1025,6 +1202,7 @@ public class HttpOutputTest
ReadableByteChannel _contentChannel;
ByteBuffer _content;
ChainedInterceptor _interceptor;
final FuturePromise<Boolean> _closedAfterWrite = new FuturePromise<>();
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
@ -1045,6 +1223,7 @@ public class HttpOutputTest
{
out.sendContent(_contentInputStream);
_contentInputStream = null;
_closedAfterWrite.succeeded(out.isClosed());
return;
}
@ -1052,6 +1231,7 @@ public class HttpOutputTest
{
out.sendContent(_contentChannel);
_contentChannel = null;
_closedAfterWrite.succeeded(out.isClosed());
return;
}
@ -1078,6 +1258,7 @@ public class HttpOutputTest
len = _arrayBuffer.length;
if (len == 0)
{
_closedAfterWrite.succeeded(out.isClosed());
async.complete();
break;
}
@ -1088,7 +1269,6 @@ public class HttpOutputTest
else
out.write(_arrayBuffer, 0, len);
}
// assertFalse(out.isReady());
}
@Override
@ -1113,7 +1293,7 @@ public class HttpOutputTest
else
out.write(_arrayBuffer, 0, len);
}
_closedAfterWrite.succeeded(out.isClosed());
return;
}
@ -1137,6 +1317,7 @@ public class HttpOutputTest
assertTrue(out.isReady());
if (BufferUtil.isEmpty(_content))
{
_closedAfterWrite.succeeded(out.isClosed());
async.complete();
break;
}
@ -1167,7 +1348,7 @@ public class HttpOutputTest
BufferUtil.flipToFlush(_byteBuffer, 0);
out.write(_byteBuffer);
}
_closedAfterWrite.succeeded(out.isClosed());
return;
}
@ -1178,6 +1359,7 @@ public class HttpOutputTest
else
out.sendContent(_content);
_content = null;
_closedAfterWrite.succeeded(out.isClosed());
return;
}
}