Fixes #2594 - Fix failing tests in SaveOptimizeTest due to timing issues (#2608)

Tests in SaveOptimizeTest were checking if the last time the SessionData had been saved was greater than it was before
if the second save occured on the same millisecond as the first it failed the assertion
now counting how many times the SessionData has been saved instead of when it was saved

Added memory barriers to TestContextScopeListener and TestSessionDataStore as they are being accessed from multiple different threads in the test

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan 2018-06-07 22:58:35 +10:00 committed by Jan Bartel
parent 0fb9022305
commit d926f04217
3 changed files with 62 additions and 65 deletions

View File

@ -20,49 +20,41 @@
package org.eclipse.jetty.server.session;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.server.handler.ContextHandler.Context;
import org.eclipse.jetty.server.handler.ContextHandler.ContextScopeListener;
public class TestContextScopeListener implements ContextScopeListener
{
CountDownLatch _exitSynchronizer;
AtomicReference<CountDownLatch> _exitSynchronizer = new AtomicReference<>();
/**
* @return the exitSynchronizer
*/
public CountDownLatch getExitSynchronizer()
{
return _exitSynchronizer;
return _exitSynchronizer.get();
}
/**
* @param exitSynchronizer the exitSynchronizer to set
*/
public void setExitSynchronizer(CountDownLatch exitSynchronizer)
{
_exitSynchronizer = exitSynchronizer;
_exitSynchronizer.set(exitSynchronizer);
}
@Override
public void enterScope(Context context, org.eclipse.jetty.server.Request request, Object reason)
{
//noop
}
@Override
public void exitScope(Context context, org.eclipse.jetty.server.Request request)
{
if (_exitSynchronizer != null)
_exitSynchronizer.countDown();
if (_exitSynchronizer.get() != null)
_exitSynchronizer.get().countDown();
}
}

View File

@ -19,10 +19,11 @@
package org.eclipse.jetty.server.session;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
/**
* TestSessionDataStore
@ -32,9 +33,10 @@ import java.util.Set;
*/
public class TestSessionDataStore extends AbstractSessionDataStore
{
public Map<String,SessionData> _map = new HashMap<>();
public boolean _passivating;
public Map<String,SessionData> _map = new ConcurrentHashMap<>();
public AtomicInteger _numSaves = new AtomicInteger(0);
public final boolean _passivating;
public TestSessionDataStore ()
{
@ -82,6 +84,7 @@ public class TestSessionDataStore extends AbstractSessionDataStore
@Override
public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
{
_numSaves.addAndGet(1);
_map.put(id, data);
}

View File

@ -19,16 +19,9 @@
package org.eclipse.jetty.server.session;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@ -37,7 +30,6 @@ import javax.servlet.http.HttpSession;
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.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.util.StringUtil;
@ -45,6 +37,15 @@ import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.junit.Test;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
/**
@ -147,28 +148,27 @@ public class SaveOptimizeTest
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
assertNotNull(sessionCookie);
String sessionId = TestServer.extractSessionId(sessionCookie);
SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(data);
long firstSaved = data.getLastSaved();
//make a few requests to access the session but not change it
int initialNumSaves = getNumSaves();
for (int i=0;i<5; i++)
{
// Perform a request to contextB with the same session cookie
Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
response = request.send();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop").send();
//check session is unchanged
SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(d);
assertEquals(firstSaved, d.getLastSaved());
assertThat(getNumSaves(), equalTo(initialNumSaves));
//slight pause between requests
Thread.currentThread().sleep(500);
Thread.sleep(500);
}
}
finally
@ -223,32 +223,29 @@ public class SaveOptimizeTest
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
assertNotNull(sessionCookie);
String sessionId = TestServer.extractSessionId(sessionCookie);
SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(data);
long lastSaved = data.getLastSaved();
// Perform a request to do nothing with the same session cookie
Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
response = request.send();
int numSavesBefore = getNumSaves();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop").send();
//check session not saved
SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(d);
assertEquals(lastSaved, d.getLastSaved());
assertThat(getNumSaves(), equalTo(numSavesBefore));
// Perform a request to mutate the session
request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=mutate");
response = request.send();
numSavesBefore = getNumSaves();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=mutate").send();
//check session is saved
d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(d);
assertTrue(d.getLastSaved() > lastSaved);
assertThat(getNumSaves(), greaterThan(numSavesBefore));
}
finally
{
@ -300,7 +297,7 @@ public class SaveOptimizeTest
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
assertNotNull(sessionCookie);
String sessionId = TestServer.extractSessionId(sessionCookie);
@ -311,8 +308,7 @@ public class SaveOptimizeTest
//make another request, session should not change
// Perform a request to do nothing with the same session cookie
Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
response = request.send();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop").send();
//check session not saved
SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
@ -320,11 +316,10 @@ public class SaveOptimizeTest
assertEquals(lastSaved, d.getLastSaved());
//wait for the savePeriod to pass and then make another request, this should save the session
Thread.currentThread().sleep(1000*savePeriod);
Thread.sleep(1000*savePeriod);
// Perform a request to do nothing with the same session cookie
request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop");
response = request.send();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop").send();
//check session is saved
d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
@ -383,7 +378,7 @@ public class SaveOptimizeTest
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
assertNotNull(sessionCookie);
String sessionId = TestServer.extractSessionId(sessionCookie);
SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
@ -392,8 +387,7 @@ public class SaveOptimizeTest
assertTrue(lastSaved > 0); //check session created was saved
// Perform a request to do nothing with the same session cookie, check the session object is different
Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop&check=diff");
response = request.send();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=noop&check=diff").send();
//check session not saved
SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
@ -453,35 +447,35 @@ public class SaveOptimizeTest
String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=create&check=true";
//make a request to set up a session on the server
int numSavesBefore = getNumSaves();
CountDownLatch latch = new CountDownLatch(1);
scopeListener.setExitSynchronizer(latch);
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
assertNotNull(sessionCookie);
String sessionId = TestServer.extractSessionId(sessionCookie);
//ensure request fully finished processing
latch.await(5, TimeUnit.SECONDS);
assertTrue(latch.await(5, TimeUnit.SECONDS));
SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(data);
long lastSaved = data.getLastSaved();
assertTrue(lastSaved > 0); //check session created was saved
//check the SessionData has been saved since before the request
assertThat(getNumSaves(), greaterThan(numSavesBefore));
// Perform a request to change maxInactive on session
numSavesBefore = getNumSaves();
latch = new CountDownLatch(1);
scopeListener.setExitSynchronizer(latch);
Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=max&value=60");
response = request.send();
client.newRequest("http://localhost:" + port1 + contextPath + servletMapping+"?action=max&value=60").send();
//ensure request fully finished processing
latch.await(5, TimeUnit.SECONDS);
assertTrue(latch.await(5, TimeUnit.SECONDS));
//check session is saved, even though the save optimisation interval has not passed
assertThat(getNumSaves(), greaterThan(numSavesBefore));
SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
assertNotNull(d);
assertTrue(d.getLastSaved() > lastSaved);
assertEquals(60000, d.getMaxInactiveMs());
}
finally
@ -519,7 +513,7 @@ public class SaveOptimizeTest
HttpSession session = request.getSession(true);
_firstSession = session;
_id = session.getId();
session.setAttribute("value", new Integer(1));
session.setAttribute("value", 1);
String check = request.getParameter("check");
if (!StringUtil.isBlank(check) && _store != null)
@ -544,7 +538,7 @@ public class SaveOptimizeTest
{
HttpSession session = request.getSession(false);
assertNotNull(session);
session.setAttribute("ttt", new Long(TimeUnit.NANOSECONDS.toMillis(System.nanoTime())));
session.setAttribute("ttt", TimeUnit.NANOSECONDS.toMillis(System.nanoTime()));
}
else if ("max".equalsIgnoreCase(action))
{
@ -572,4 +566,12 @@ public class SaveOptimizeTest
}
}
private int getNumSaves()
{
assertNotNull(_servlet);
assertNotNull(_servlet._store);
assertTrue(TestSessionDataStore.class.isInstance(_servlet._store));
return ((TestSessionDataStore)_servlet._store)._numSaves.get();
}
}