Merge pull request #3205 from eclipse/jetty-9.4.x-3202-async-session-create

Issue #3202 Ensure sessions created during async are cleaned up.
This commit is contained in:
Greg Wilkins 2018-12-18 16:43:20 +11:00 committed by GitHub
commit 28533d4ca6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 490 additions and 135 deletions

View File

@ -58,32 +58,7 @@ public class AsyncContextState implements AsyncContext
@Override
public void addListener(final AsyncListener listener, final ServletRequest request, final ServletResponse response)
{
AsyncListener wrap = new AsyncListener()
{
@Override
public void onTimeout(AsyncEvent event) throws IOException
{
listener.onTimeout(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable()));
}
@Override
public void onStartAsync(AsyncEvent event) throws IOException
{
listener.onStartAsync(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable()));
}
@Override
public void onError(AsyncEvent event) throws IOException
{
listener.onError(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable()));
}
@Override
public void onComplete(AsyncEvent event) throws IOException
{
listener.onComplete(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable()));
}
};
AsyncListener wrap = new WrappedAsyncListener(listener, request, response);
state().addListener(wrap);
}
@ -188,6 +163,46 @@ public class AsyncContextState implements AsyncContext
return state();
}
public static class WrappedAsyncListener implements AsyncListener
{
private final AsyncListener _listener;
private final ServletRequest _request;
private final ServletResponse _response;
public WrappedAsyncListener(AsyncListener listener, ServletRequest request, ServletResponse response)
{
_listener = listener;
_request = request;
_response = response;
}
public AsyncListener getListener()
{
return _listener;
}
@Override
public void onTimeout(AsyncEvent event) throws IOException
{
_listener.onTimeout(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable()));
}
@Override
public void onStartAsync(AsyncEvent event) throws IOException
{
_listener.onStartAsync(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable()));
}
@Override
public void onError(AsyncEvent event) throws IOException
{
_listener.onError(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable()));
}
@Override
public void onComplete(AsyncEvent event) throws IOException
{
_listener.onComplete(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable()));
}
}
}

View File

@ -149,6 +149,25 @@ public class HttpChannelState
}
}
public boolean hasListener(AsyncListener listener)
{
try(Locker.Lock lock= _locker.lock())
{
if (_asyncListeners==null)
return false;
for (AsyncListener l : _asyncListeners)
{
if (l==listener)
return true;
if (l instanceof AsyncContextState.WrappedAsyncListener && ((AsyncContextState.WrappedAsyncListener)l).getListener()==listener)
return true;
}
return false;
}
}
public void setTimeout(long ms)
{
try(Locker.Lock lock= _locker.lock())

View File

@ -55,7 +55,7 @@ public class DefaultSessionIdManager extends ContainerLifeCycle implements Sessi
{
private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session");
private final static String __NEW_SESSION_ID="org.eclipse.jetty.server.newSessionId";
public final static String __NEW_SESSION_ID="org.eclipse.jetty.server.newSessionId";
protected static final AtomicLong COUNTER = new AtomicLong();

View File

@ -1148,4 +1148,18 @@ public class Session implements SessionHandler.SessionIf
return _resident;
}
@Override
public String toString()
{
try (Lock lock = _lock.lock())
{
return String.format("%s@%x{id=%s,x=%s,req=%d,res=%b}",
getClass().getSimpleName(),
hashCode(),
_sessionData.getId(),
_extendedId,
_requests,
_resident);
}
}
}

View File

@ -162,8 +162,22 @@ public class SessionHandler extends ScopedHandler
@Override
public void onComplete(AsyncEvent event) throws IOException
{
//An async request has completed, so we can complete the session
complete(Request.getBaseRequest(event.getAsyncContext().getRequest()).getSession(false));
// An async request has completed, so we can complete the session,
// but we must locate the session instance for this context
Request request = Request.getBaseRequest(event.getAsyncContext().getRequest());
HttpSession session = request.getSession(false);
String id;
if (session!=null)
id = session.getId();
else
{
id = (String)request.getAttribute(DefaultSessionIdManager.__NEW_SESSION_ID);
if (id==null)
id = request.getRequestedSessionId();
}
if (id!=null)
complete(getSession(id));
}
@Override
@ -407,9 +421,12 @@ public class SessionHandler extends ScopedHandler
*/
public void complete(HttpSession session)
{
if (LOG.isDebugEnabled())
LOG.debug("Complete called with session {}", session);
if (session == null)
return;
Session s = ((SessionIf)session).getSession();
try
@ -422,23 +439,26 @@ public class SessionHandler extends ScopedHandler
LOG.warn(e);
}
}
public void complete (Session session, Request request)
@Deprecated
public void complete(Session session, Request baseRequest)
{
if (request.isAsyncStarted() && request.getDispatcherType() == DispatcherType.REQUEST)
ensureCompletion(baseRequest);
}
private void ensureCompletion(Request baseRequest)
{
if (baseRequest.isAsyncStarted())
{
request.getAsyncContext().addListener(_sessionAsyncListener);
if (LOG.isDebugEnabled())
LOG.debug("Adding AsyncListener for {}", baseRequest);
if (!baseRequest.getHttpChannelState().hasListener(_sessionAsyncListener))
baseRequest.getAsyncContext().addListener(_sessionAsyncListener);
}
else
{
complete(session);
complete(baseRequest.getSession(false));
}
//if dispatcher type is not async and not request, complete immediately (its a forward or an include)
//else if dispatcher type is request and not async, complete immediately
//else register an async callback completion listener that will complete the session
}
@ -455,7 +475,6 @@ public class SessionHandler extends ScopedHandler
_context=ContextHandler.getCurrentContext();
_loader=Thread.currentThread().getContextClassLoader();
synchronized (server)
{
//Get a SessionDataStore and a SessionDataStore, falling back to in-memory sessions only
@ -472,7 +491,6 @@ public class SessionHandler extends ScopedHandler
_sessionCache.setSessionDataStore(sds);
}
if (_sessionIdManager==null)
{
@ -1593,16 +1611,19 @@ public class SessionHandler extends ScopedHandler
@Override
public void doScope(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
SessionHandler old_session_manager = null;
SessionHandler old_session_handler = null;
HttpSession old_session = null;
HttpSession existingSession = null;
try
{
old_session_manager = baseRequest.getSessionHandler();
if (LOG.isDebugEnabled())
LOG.debug("SessionHandler.doScope");
old_session_handler = baseRequest.getSessionHandler();
old_session = baseRequest.getSession(false);
if (old_session_manager != this)
if (old_session_handler != this)
{
// new session context
baseRequest.setSessionHandler(this);
@ -1613,7 +1634,7 @@ public class SessionHandler extends ScopedHandler
// access any existing session for this context
existingSession = baseRequest.getSession(false);
if ((existingSession != null) && (old_session_manager != this))
if ((existingSession != null) && (old_session_handler != this))
{
HttpCookie cookie = access(existingSession,request.isSecure());
// Handle changed ID or max-age refresh, but only if this is not a redispatched request
@ -1622,10 +1643,7 @@ public class SessionHandler extends ScopedHandler
}
if (LOG.isDebugEnabled())
{
LOG.debug("sessionHandler=" + this);
LOG.debug("session=" + existingSession);
}
LOG.debug("sessionHandler={} session={}",this, existingSession);
if (_nextScope != null)
_nextScope.doScope(target,baseRequest,request,response);
@ -1637,16 +1655,18 @@ public class SessionHandler extends ScopedHandler
finally
{
//if there is a session that was created during handling this context, then complete it
HttpSession finalSession = baseRequest.getSession(false);
if (LOG.isDebugEnabled()) LOG.debug("FinalSession="+finalSession+" old_session_manager="+old_session_manager+" this="+this);
if ((finalSession != null) && (old_session_manager != this))
if (LOG.isDebugEnabled())
LOG.debug("FinalSession={}, old_session_handler={}, this={}, calling complete={}", baseRequest.getSession(false), old_session_handler, this, (old_session_handler != this));
// If we are leaving the scope of this session handler, ensure the session is completed
if (old_session_handler != this)
ensureCompletion(baseRequest);
// revert the session handler to the previous, unless it was null, in which case remember it as
// the first session handler encountered.
if (old_session_handler != null && old_session_handler != this)
{
complete((Session)finalSession, baseRequest);
}
if (old_session_manager != null && old_session_manager != this)
{
baseRequest.setSessionHandler(old_session_manager);
baseRequest.setSessionHandler(old_session_handler);
baseRequest.setSession(old_session);
}
}

View File

@ -117,7 +117,7 @@ public class AttributeNameTest
//Mangle the cookie, replacing Path with $Path, etc.
sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=","$1\\$Path=");
//Make a request to the 2nd server which will do a refresh, use TestFooServlet to ensure that the
//Make a request to the 2nd server which will do a refresh, use TestServlet to ensure that the
//session attribute with dotted name is not removed
Request request2 = client.newRequest("http://localhost:" + port2 + contextPath + servletMapping + "?action=get");
request2.header("Cookie", sessionCookie);

View File

@ -1,59 +0,0 @@
//
// ========================================================================
// Copyright (c) 1995-2018 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 java.io.IOException;
import java.lang.reflect.Proxy;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
public class TestFooServlet 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);
TestFoo testFoo = new TestFoo();
testFoo.setInt(33);
FooInvocationHandler handler = new FooInvocationHandler(testFoo);
Foo foo = (Foo)Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class[] {Foo.class}, handler);
session.setAttribute("foo", foo);
}
else if ("test".equals(action))
{
HttpSession session = request.getSession(false);
if (session == null)
response.sendError(500, "Session not activated");
Foo foo = (Foo)session.getAttribute("foo");
if (foo == null || foo.getInt() != 33)
response.sendError(500, "Foo not deserialized");
}
}
}

View File

@ -0,0 +1,358 @@
//
// ========================================================================
// Copyright (c) 1995-2018 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.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.lang.reflect.Proxy;
import javax.servlet.AsyncContext;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import javax.servlet.WriteListener;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.junit.jupiter.api.Test;
/**
* AsyncTest
*
* Tests async handling wrt sessions.
*/
public class AsyncTest
{
public static class LatchServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.getWriter().println("Latched");
}
}
@Test
public void testSessionWithAsyncDispatch() throws Exception
{
// Test async dispatch back to same context, which then creates a session.
DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
String contextPath = "";
String mapping = "/server";
ServletContextHandler contextHandler = server.addContext(contextPath);
TestServlet servlet = new TestServlet();
ServletHolder holder = new ServletHolder(servlet);
contextHandler.addServlet(holder, mapping);
LatchServlet latchServlet = new LatchServlet();
ServletHolder latchHolder = new ServletHolder(latchServlet);
contextHandler.addServlet(latchHolder, "/latch");
server.start();
int port = server.getPort();
try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
HttpClient client = new HttpClient();
client.start();
String url = "http://localhost:" + port + contextPath + mapping+"?action=async";
//make a request to set up a session on the server
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
//make another request, when this is handled, the first request is definitely finished being handled
response = client.GET("http://localhost:" + port + contextPath + "/latch");
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
//session should now be evicted from the cache after request exited
String id = TestServer.extractSessionId(sessionCookie);
assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
}
finally
{
server.stop();
}
}
@Test
public void testSessionWithAsyncComplete() throws Exception
{
// Test async write, which creates a session and completes outside of a dispatch
DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
String contextPath = "";
String mapping = "/server";
ServletContextHandler contextHandler = server.addContext(contextPath);
TestServlet servlet = new TestServlet();
ServletHolder holder = new ServletHolder(servlet);
contextHandler.addServlet(holder, mapping);
LatchServlet latchServlet = new LatchServlet();
ServletHolder latchHolder = new ServletHolder(latchServlet);
contextHandler.addServlet(latchHolder, "/latch");
server.start();
int port = server.getPort();
try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
HttpClient client = new HttpClient();
client.start();
String url = "http://localhost:" + port + contextPath + mapping+"?action=asyncComplete";
//make a request to set up a session on the server
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
//make another request, when this is handled, the first request is definitely finished being handled
response = client.GET("http://localhost:" + port + contextPath + "/latch");
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
//session should now be evicted from the cache after request exited
String id = TestServer.extractSessionId(sessionCookie);
assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
}
finally
{
server.stop();
}
}
@Test
public void testSessionWithCrossContextAsync() throws Exception
{
// Test async dispatch from context A to context B then
// async dispatch back to context B, which then creates a session (in context B).
DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
ServletContextHandler contextA = server.addContext("/ctxA");
CrossContextServlet ccServlet = new CrossContextServlet();
ServletHolder ccHolder = new ServletHolder(ccServlet);
contextA.addServlet(ccHolder, "/*");
ServletContextHandler contextB = server.addContext("/ctxB");
TestServlet testServlet = new TestServlet();
ServletHolder testHolder = new ServletHolder(testServlet);
contextB.addServlet(testHolder, "/*");
LatchServlet latchServlet = new LatchServlet();
ServletHolder latchHolder = new ServletHolder(latchServlet);
contextB.addServlet(latchHolder, "/latch");
server.start();
int port = server.getPort();
try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
HttpClient client = new HttpClient();
client.start();
String url = "http://localhost:" + port + "/ctxA/test?action=async";
//make a request to set up a session on the server
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
//make another request, when this is handled, the first request is definitely finished being handled
response = client.GET("http://localhost:" + port + "/ctxB/latch");
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
//session should now be evicted from the cache after request exited
String id = TestServer.extractSessionId(sessionCookie);
assertFalse(contextB.getSessionHandler().getSessionCache().contains(id));
assertTrue(contextB.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
}
finally
{
server.stop();
}
}
@Test
public void testSessionWithCrossContextAsyncComplete() throws Exception
{
// Test async dispatch from context A to context B, which then does an
// async write, which creates a session (in context A) and completes outside of a
// dispatch
DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
ServletContextHandler contextA = server.addContext("/ctxA");
CrossContextServlet ccServlet = new CrossContextServlet();
ServletHolder ccHolder = new ServletHolder(ccServlet);
contextA.addServlet(ccHolder, "/*");
ServletContextHandler contextB = server.addContext("/ctxB");
TestServlet testServlet = new TestServlet();
ServletHolder testHolder = new ServletHolder(testServlet);
contextB.addServlet(testHolder, "/*");
LatchServlet latchServlet = new LatchServlet();
ServletHolder latchHolder = new ServletHolder(latchServlet);
contextB.addServlet(latchHolder, "/latch");
server.start();
int port = server.getPort();
try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
HttpClient client = new HttpClient();
client.start();
String url = "http://localhost:" + port + "/ctxA/test?action=asyncComplete";
//make a request to set up a session on the server
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
//make another request, when this is handled, the first request is definitely finished being handled
response = client.GET("http://localhost:" + port + "/ctxB/latch");
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
//session should now be evicted from the cache A after request exited
String id = TestServer.extractSessionId(sessionCookie);
assertFalse(contextA.getSessionHandler().getSessionCache().contains(id));
assertTrue(contextA.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
}
finally
{
server.stop();
}
}
public static class TestServlet 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);
TestFoo testFoo = new TestFoo();
testFoo.setInt(33);
FooInvocationHandler handler = new FooInvocationHandler(testFoo);
Foo foo = (Foo)Proxy
.newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class[] {Foo.class}, handler);
session.setAttribute("foo", foo);
}
else if ("test".equals(action))
{
HttpSession session = request.getSession(false);
if (session == null)
response.sendError(500, "Session not activated");
Foo foo = (Foo)session.getAttribute("foo");
if (foo == null || foo.getInt() != 33)
response.sendError(500, "Foo not deserialized");
}
else if ("async".equals(action))
{
if (request.getAttribute("async-test") == null)
{
request.setAttribute("async-test", Boolean.TRUE);
AsyncContext acontext = request.startAsync();
acontext.dispatch();
return;
}
else
{
HttpSession session = request.getSession(true);
response.getWriter().println("OK");
}
}
else if ("asyncComplete".equals(action))
{
AsyncContext acontext = request.startAsync();
ServletOutputStream out = response.getOutputStream();
out.setWriteListener(new WriteListener()
{
@Override
public void onWritePossible() throws IOException
{
if (out.isReady())
{
request.getSession(true);
out.print("OK\n");
acontext.complete();
}
}
@Override
public void onError(Throwable t)
{
}
});
}
}
}
public static class CrossContextServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
AsyncContext acontext = request.startAsync();
acontext.dispatch(request.getServletContext().getContext("/ctxB"),"/test");
}
}
}

View File

@ -128,12 +128,7 @@ public class DeleteUnloadableSessionTest
}
}
/**
* TestFooServlet
*
*
*/
public static class TestServlet extends HttpServlet
{
private static final long serialVersionUID = 1L;

View File

@ -136,16 +136,9 @@ public class SessionEvictionFailureTest
}
/**
* TestFooServlet
*
*
*/
public static class TestServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException
{