details: https://code.openbravo.com/erp/devel/pi/rev/ca9d0d95c9b7
changeset: 33451:ca9d0d95c9b7
user: Asier Lostalé <asier.lostale <at> openbravo.com>
date: Thu Feb 15 14:37:32 2018 +0100
summary: fiexed bug 37919: contention on session creation, deactivation and
check
Replaced HashSet that required to be externally synchronized with a Set backed
with a ConcurrentHashMap which better handles synchronization.
diffstat:
src-test/src/org/openbravo/test/system/Sessions.java | 11 ++++-
src/org/openbravo/erpCommon/security/SessionListener.java | 32 ++++++--------
2 files changed, 23 insertions(+), 20 deletions(-)
diffs (149 lines):
diff -r 037253b579a3 -r ca9d0d95c9b7
src-test/src/org/openbravo/test/system/Sessions.java
--- a/src-test/src/org/openbravo/test/system/Sessions.java Thu Feb 15
13:01:08 2018 +0100
+++ b/src-test/src/org/openbravo/test/system/Sessions.java Thu Feb 15
14:37:32 2018 +0100
@@ -19,7 +19,7 @@
package org.openbravo.test.system;
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import java.util.ArrayList;
@@ -37,21 +37,27 @@
import org.junit.Test;
import org.openbravo.erpCommon.security.SessionListener;
import org.openbravo.erpCommon.utility.SequenceIdData;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/** Test cases covering Session management */
public class Sessions {
+ private static final Logger log = LoggerFactory.getLogger(Sessions.class);
private static int NUMBER_OF_THREADS = 4;
private static int NUMBER_OF_SESSIONS_PER_THREAD = 1_000;
/** Covers bug #37893 */
@Test
public void canCreateAndCheckSessionsInParallel() throws
InterruptedException, ExecutionException {
+ log.info("Starting {} threads to create {} sessions each",
NUMBER_OF_THREADS,
+ NUMBER_OF_SESSIONS_PER_THREAD);
SessionListener sl = new SessionListener();
List<SessionCreatorAndChecker> tasks = new ArrayList<>();
for (int i = 0; i < NUMBER_OF_THREADS; i++) {
tasks.add(new SessionCreatorAndChecker(sl));
}
+ long t = System.currentTimeMillis();
ExecutorService es = Executors.newFixedThreadPool(NUMBER_OF_THREADS);
List<Future<Void>> executions = es.invokeAll(tasks);
@@ -59,6 +65,7 @@
// if assertion failed, this will throw exception
e.get();
}
+ log.info("All finished in {} ms", System.currentTimeMillis() - t);
}
private class SessionCreatorAndChecker implements Callable<Void> {
@@ -70,6 +77,7 @@
@Override
public Void call() {
+ long t = System.currentTimeMillis();
for (int i = 0; i < NUMBER_OF_SESSIONS_PER_THREAD; i++) {
String sessionId = SequenceIdData.getUUID();
@@ -84,6 +92,7 @@
assertThat(sessionFromContext, is(s));
}
+ log.info("Thread finished in {} ms", System.currentTimeMillis() - t);
return null;
}
}
diff -r 037253b579a3 -r ca9d0d95c9b7
src/org/openbravo/erpCommon/security/SessionListener.java
--- a/src/org/openbravo/erpCommon/security/SessionListener.java Thu Feb 15
13:01:08 2018 +0100
+++ b/src/org/openbravo/erpCommon/security/SessionListener.java Thu Feb 15
14:37:32 2018 +0100
@@ -21,8 +21,9 @@
import java.text.SimpleDateFormat;
import java.util.Calendar;
-import java.util.HashSet;
+import java.util.Collections;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.servlet.ServletContext;
import javax.servlet.ServletContextEvent;
@@ -48,8 +49,10 @@
private static final Logger log = Logger.getLogger(SessionListener.class);
- private static Set<String> sessionsInContext = new HashSet<String>();
- private static Set<HttpSession> activeHttpSessions = new
HashSet<HttpSession>();
+ private static Set<String> sessionsInContext = Collections
+ .newSetFromMap(new ConcurrentHashMap<String, Boolean>());
+ private static Set<HttpSession> activeHttpSessions = Collections
+ .newSetFromMap(new ConcurrentHashMap<HttpSession, Boolean>());
private static ServletContext context = null;
/**
@@ -64,10 +67,7 @@
if (sessionId != null) {
deactivateSession(sessionId);
}
- synchronized (activeHttpSessions) {
- activeHttpSessions.remove(session);
- }
-
+ activeHttpSessions.remove(session);
log.debug("Session destroyed. Active sessions count: " +
activeHttpSessions.size());
}
@@ -102,7 +102,7 @@
* @param sessionId
* db id for the session to keep track
*/
- public static synchronized void addSession(String sessionId) {
+ public static void addSession(String sessionId) {
sessionsInContext.add(sessionId);
}
@@ -155,9 +155,7 @@
@Override
public void sessionCreated(HttpSessionEvent event) {
- synchronized (activeHttpSessions) {
- activeHttpSessions.add(event.getSession());
- }
+ activeHttpSessions.add(event.getSession());
if (RequestContext.get().getRequest() != null
&&
AuthenticationManager.isStatelessRequest(RequestContext.get().getRequest())) {
@@ -175,11 +173,9 @@
*/
public static HttpSession getActiveSession(String sessionId) {
try {
- synchronized (activeHttpSessions) {
- for (HttpSession session : activeHttpSessions) {
- if (sessionId.equals(session.getAttribute("#AD_SESSION_ID"))) {
- return session;
- }
+ for (HttpSession session : activeHttpSessions) {
+ if (sessionId.equals(session.getAttribute("#AD_SESSION_ID"))) {
+ return session;
}
}
} catch (Exception e) {
@@ -191,9 +187,7 @@
private void deactivateSession(String sessionId) {
try {
- synchronized (sessionsInContext) {
- sessionsInContext.remove(sessionId);
- }
+ sessionsInContext.remove(sessionId);
// Do not use DAL here
SessionLoginData.deactivate((ConnectionProvider)
context.getAttribute("openbravoPool"),
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openbravo-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openbravo-commits
