From d703319c874e784965c66c143fd31dbe8afbe5da Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 17 May 2013 16:35:58 +1000 Subject: [PATCH 1/2] 398467 Servlet 3.1 Non Blocking IO simplified write --- .../org/eclipse/jetty/server/HttpOutput.java | 59 +++++++++---------- .../server/HttpManyWaysToCommitTest.java | 17 +++--- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index a918541e041..86665bbbaf6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -155,59 +155,54 @@ public class HttpOutput extends ServletOutputStream @Override public void write(byte[] b, int off, int len) throws IOException - { + { if (isClosed()) throw new EOFException("Closed"); - // Do we have an aggregate buffer ? - if (_aggregate == null) - { - // NO - should we have an aggregate buffer? yes if this write will easily fit in it - int size = getBufferSize(); - if (len<=size/2) - { - _aggregate = _channel.getByteBufferPool().acquire(size, false); - BufferUtil.append(_aggregate, b, off, len); - _written += len; - closeIfAllContentWritten(); - return; - } - } - else + _written+=len; + boolean complete=_channel.getResponse().isAllContentWritten(_written); + int capacity = getBufferSize(); + + // Should we aggregate? + if (!complete && len<=capacity/4) { + if (_aggregate == null) + _aggregate = _channel.getByteBufferPool().acquire(capacity, false); + // YES - fill the aggregate with content from the buffer int filled = BufferUtil.fill(_aggregate, b, off, len); - _written += filled; - - // if closed or there is no content left over and we are not full, then we are done - if (closeIfAllContentWritten() || filled==len && !BufferUtil.isFull(_aggregate)) + + // return if we are not complete, not full and filled all the content + if (!complete && filled==len && !BufferUtil.isFull(_aggregate)) return; - + + // adjust offset/length off+=filled; len-=filled; } - - // flush the aggregate + + // flush any content from the aggregate if (BufferUtil.hasContent(_aggregate)) { - _channel.write(_aggregate, false); - + _channel.write(_aggregate, complete && len==0); + // should we fill aggregate again from the buffer? - if (len<_aggregate.capacity()/2) + if (len>0 && !complete && len<=_aggregate.capacity()/4) { BufferUtil.append(_aggregate, b, off, len); - _written += len; - closeIfAllContentWritten(); return; } } - + // write any remaining content in the buffer directly - _written += len; - boolean complete=_channel.getResponse().isAllContentWritten(_written); - _channel.write(ByteBuffer.wrap(b, off, len), complete); + if (len>0) + _channel.write(ByteBuffer.wrap(b, off, len), complete); + if (complete) + { + closed(); _channel.getResponse().closeOutput(); + } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java index c567873b91b..cfc255f92ed 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java @@ -327,7 +327,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest SimpleHttpResponse response = executeRequest(); assertThat("response code is 200", response.getCode(), is("200")); - assertResponseBody(response, "foobar"); + assertResponseBody(response, "foobarfoobar"); if (!"HTTP/1.0".equals(httpVersion)) assertHeader(response, "transfer-encoding", "chunked"); } @@ -341,7 +341,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest SimpleHttpResponse response = executeRequest(); assertThat("response code is 200", response.getCode(), is("200")); - assertResponseBody(response, "foobar"); + assertResponseBody(response, "foobarfoobar"); if (!"HTTP/1.0".equals(httpVersion)) assertHeader(response, "transfer-encoding", "chunked"); } @@ -372,7 +372,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { baseRequest.setHandled(true); - response.setBufferSize(3); + response.setBufferSize(4); response.getWriter().write("foobar"); super.handle(target, baseRequest, request, response); } @@ -389,9 +389,9 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { baseRequest.setHandled(true); - response.setBufferSize(3); - response.getWriter().write("f"); - response.getWriter().write("oobar"); + response.setBufferSize(8); + response.getWriter().write("fo"); + response.getWriter().write("obarfoobar"); super.handle(target, baseRequest, request, response); } } @@ -407,7 +407,10 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { baseRequest.setHandled(true); - response.setBufferSize(5); + response.setBufferSize(8); + response.getWriter().write("fo"); + response.getWriter().write("ob"); + response.getWriter().write("ar"); response.getWriter().write("fo"); response.getWriter().write("ob"); response.getWriter().write("ar"); From b6d1158c2232d87701a3c41c15d8c0131c9e1085 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 17 May 2013 16:17:11 +1000 Subject: [PATCH 2/2] 408167 JDBCSessionManager don't mark session as dirty if same attribute value set --- .../jetty/server/session/AbstractSession.java | 9 +- .../server/session/JDBCSessionManager.java | 3 +- .../server/session/DirtyAttributeTest.java | 225 ++++++++++++++++++ 3 files changed, 234 insertions(+), 3 deletions(-) create mode 100644 tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/DirtyAttributeTest.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java index 9830df24f2c..db6b465c5eb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java @@ -479,6 +479,12 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI /* ------------------------------------------------------------ */ @Override public void setAttribute(String name, Object value) + { + updateAttribute(name,value); + } + + /* ------------------------------------------------------------ */ + protected boolean updateAttribute (String name, Object value) { Object old=null; synchronized (this) @@ -495,8 +501,9 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI bindValue(name,value); _manager.doSessionAttributeListeners(this,name,old,value); - + return true; } + return false; } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java index e596188fe47..bf595d15bb2 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java @@ -244,8 +244,7 @@ public class JDBCSessionManager extends AbstractSessionManager @Override public void setAttribute (String name, Object value) { - super.setAttribute(name, value); - _dirty=true; + _dirty=updateAttribute(name, value); } @Override diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/DirtyAttributeTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/DirtyAttributeTest.java new file mode 100644 index 00000000000..ac94ec4e1ec --- /dev/null +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/DirtyAttributeTest.java @@ -0,0 +1,225 @@ +// +// ======================================================================== +// Copyright (c) 1995-2013 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server.session; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.io.Serializable; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import javax.servlet.http.HttpSessionActivationListener; +import javax.servlet.http.HttpSessionBindingEvent; +import javax.servlet.http.HttpSessionBindingListener; +import javax.servlet.http.HttpSessionEvent; + +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.junit.Test; + + +/** + * DirtyAttributeTest + * + * Check that repeated calls to setAttribute with the same value do not cause writes to the + * database - this is inferred via calls to passivate and activate listeners. + * + * + */ +public class DirtyAttributeTest +{ + public static TestValue A_VALUE = new TestValue(); + public static TestValue B_VALUE = new TestValue(); + public static String THE_NAME = "__theName"; + public static int INACTIVE = 4; + public static int SCAVENGE = 1; + + @Test + public void testDiryWrite() throws Exception + { + AbstractTestServer server = new JdbcTestServer(0,INACTIVE,SCAVENGE); + + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestDirtyServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + ContentResponse response = client.GET("http://localhost:" + port + "/mod/test?action=create"); + + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + String sessionCookie = response.getHeaders().getStringField("Set-Cookie"); + assertTrue(sessionCookie != null); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request to change the session attribute + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=setA"); + request.header("Cookie", sessionCookie); + response = request.send(); + + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + A_VALUE.assertPassivatesEquals(1); + A_VALUE.assertActivatesEquals(1); + A_VALUE.assertBindsEquals(1); + A_VALUE.assertUnbindsEquals(0); + + //do another request using the cookie to try changing the session attribute to the same value again + request= client.newRequest("http://localhost:" + port + "/mod/test?action=setA"); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + A_VALUE.assertPassivatesEquals(1); + A_VALUE.assertActivatesEquals(1); + A_VALUE.assertBindsEquals(1); + A_VALUE.assertUnbindsEquals(0); + + //do another request using the cookie and change to a different value + request= client.newRequest("http://localhost:" + port + "/mod/test?action=setB"); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + B_VALUE.assertPassivatesEquals(1); + B_VALUE.assertActivatesEquals(1); + B_VALUE.assertBindsEquals(1); + B_VALUE.assertUnbindsEquals(0); + A_VALUE.assertBindsEquals(1); + A_VALUE.assertUnbindsEquals(1); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + + public static class TestValue implements HttpSessionActivationListener, HttpSessionBindingListener, Serializable + { + int passivates = 0; + int activates = 0; + int binds = 0; + int unbinds = 0; + + /** + * @see javax.servlet.http.HttpSessionActivationListener#sessionWillPassivate(javax.servlet.http.HttpSessionEvent) + */ + public void sessionWillPassivate(HttpSessionEvent se) + { + ++passivates; + } + + /** + * @see javax.servlet.http.HttpSessionActivationListener#sessionDidActivate(javax.servlet.http.HttpSessionEvent) + */ + public void sessionDidActivate(HttpSessionEvent se) + { + ++activates; + } + + public void assertPassivatesEquals (int expected) + { + assertEquals(expected, passivates); + } + + public void assertActivatesEquals (int expected) + { + assertEquals(expected, activates); + } + + public void assertBindsEquals (int expected) + { + assertEquals(expected, binds); + } + + public void assertUnbindsEquals (int expected) + { + assertEquals(expected, unbinds); + } + + + /** + * @see javax.servlet.http.HttpSessionBindingListener#valueBound(javax.servlet.http.HttpSessionBindingEvent) + */ + public void valueBound(HttpSessionBindingEvent event) + { + ++binds; + } + + /** + * @see javax.servlet.http.HttpSessionBindingListener#valueUnbound(javax.servlet.http.HttpSessionBindingEvent) + */ + public void valueUnbound(HttpSessionBindingEvent event) + { + ++unbinds; + } + } + + public static class TestDirtyServlet extends HttpServlet + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + String action = request.getParameter("action"); + + if ("create".equals(action)) + { + HttpSession session = request.getSession(true); + return; + } + + if ("setA".equals(action)) + { + HttpSession session = request.getSession(false); + if (session == null) + throw new ServletException("Session is null for action=change"); + + session.setAttribute(THE_NAME, A_VALUE); + return; + } + + if ("setB".equals(action)) + { + HttpSession session = request.getSession(false); + if (session == null) + throw new ServletException("Session does not exist"); + session.setAttribute(THE_NAME, B_VALUE); + return; + } + } + } + +}