You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2017/04/25 18:29:45 UTC
[09/11] geode git commit: GEODE-2808 - Fixing lock ordering issues in
DeltaSession
GEODE-2808 - Fixing lock ordering issues in DeltaSession
Region expiration of sessions and explicit expiration of sessions had
lock ordering issues. Fixing the code so that expiration goes through
the region entry lock first, before getting the lock on StandardSession.
Adding a workaround for the fact that liferay calls removeAttribute
from within session expiration by ignoreing remoteAttribute calls during
expiration.
This closes #472
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/45dc6744
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/45dc6744
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/45dc6744
Branch: refs/heads/feature/GEODE-2801
Commit: 45dc6744154a08986e833852ef743af5d8bf19ba
Parents: 47d8c82
Author: Dan Smith <up...@apache.org>
Authored: Fri Apr 21 11:36:24 2017 -0700
Committer: Dan Smith <up...@apache.org>
Committed: Fri Apr 21 16:23:24 2017 -0700
----------------------------------------------------------------------
.../modules/session/catalina/DeltaSession7.java | 14 +++++++-
.../modules/session/catalina/DeltaSession8.java | 14 +++++++-
.../session/TestSessionsTomcat8Base.java | 34 ++++++++++++++++++++
.../modules/session/catalina/DeltaSession.java | 14 +++++++-
.../geode/modules/session/CommandServlet.java | 4 +++
.../geode/modules/session/QueryCommand.java | 2 ++
.../geode/modules/session/TestSessionsBase.java | 34 ++++++++++++++++++++
7 files changed, 113 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules-tomcat7/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession7.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules-tomcat7/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession7.java b/extensions/geode-modules-tomcat7/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession7.java
index 204ff5e..d7f30bd 100644
--- a/extensions/geode-modules-tomcat7/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession7.java
+++ b/extensions/geode-modules-tomcat7/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession7.java
@@ -263,6 +263,9 @@ public class DeltaSession7 extends StandardSession
public void removeAttribute(String name, boolean notify) {
checkBackingCacheAvailable();
+ if (expired) {
+ return;
+ }
synchronized (this.changeLock) {
// Remove the attribute locally
super.removeAttribute(name, true);
@@ -322,7 +325,7 @@ public class DeltaSession7 extends StandardSession
setExpired(true);
// Do expire processing
- expire();
+ super.expire(true);
// Update statistics
if (manager != null) {
@@ -330,6 +333,15 @@ public class DeltaSession7 extends StandardSession
}
}
+ @Override
+ public void expire(boolean notify) {
+ if (notify) {
+ getOperatingRegion().destroy(this.getId(), this);
+ } else {
+ super.expire(false);
+ }
+ }
+
public void setMaxInactiveInterval(int interval) {
super.setMaxInactiveInterval(interval);
}
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules-tomcat8/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession8.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules-tomcat8/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession8.java b/extensions/geode-modules-tomcat8/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession8.java
index b5e7d0c..f69382a 100644
--- a/extensions/geode-modules-tomcat8/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession8.java
+++ b/extensions/geode-modules-tomcat8/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession8.java
@@ -258,6 +258,9 @@ public class DeltaSession8 extends StandardSession
public void removeAttribute(String name, boolean notify) {
checkBackingCacheAvailable();
+ if (expired) {
+ return;
+ }
synchronized (this.changeLock) {
// Remove the attribute locally
super.removeAttribute(name, true);
@@ -317,7 +320,7 @@ public class DeltaSession8 extends StandardSession
setExpired(true);
// Do expire processing
- expire();
+ super.expire(true);
// Update statistics
if (manager != null) {
@@ -325,6 +328,15 @@ public class DeltaSession8 extends StandardSession
}
}
+ @Override
+ public void expire(boolean notify) {
+ if (notify) {
+ getOperatingRegion().destroy(this.getId(), this);
+ } else {
+ super.expire(false);
+ }
+ }
+
public void setMaxInactiveInterval(int interval) {
super.setMaxInactiveInterval(interval);
}
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/TestSessionsTomcat8Base.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/TestSessionsTomcat8Base.java b/extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/TestSessionsTomcat8Base.java
index 15b3874..1dc1d8b 100644
--- a/extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/TestSessionsTomcat8Base.java
+++ b/extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/TestSessionsTomcat8Base.java
@@ -233,6 +233,40 @@ public abstract class TestSessionsTomcat8Base extends JUnit4DistributedTestCase
}
/**
+ * Test expiration of a session by the tomcat container, rather than gemfire expiration
+ */
+ @Test
+ public void testSessionExpirationByContainer() throws Exception {
+
+ String key = "value_testSessionExpiration1";
+ String value = "Foo";
+
+ WebConversation wc = new WebConversation();
+ WebRequest req = new GetMethodWebRequest(String.format("http://localhost:%d/test", port));
+
+ // Set an attribute
+ req.setParameter("cmd", QueryCommand.SET.name());
+ req.setParameter("param", key);
+ req.setParameter("value", value);
+ WebResponse response = wc.getResponse(req);
+
+ // Set the session timeout of this one session.
+ req.setParameter("cmd", QueryCommand.SET_MAX_INACTIVE.name());
+ req.setParameter("value", "1");
+ response = wc.getResponse(req);
+
+ // Wait until the session should expire
+ Thread.sleep(2000);
+
+ // Do a request, which should cause the session to be expired
+ req.setParameter("cmd", QueryCommand.GET.name());
+ req.setParameter("param", key);
+ response = wc.getResponse(req);
+
+ assertEquals("", response.getText());
+ }
+
+ /**
* Test that removing a session attribute also removes it from the region
*/
@Test
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
index bc421a5..ac612da 100644
--- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
+++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
@@ -266,6 +266,9 @@ public class DeltaSession extends StandardSession
public void removeAttribute(String name, boolean notify) {
checkBackingCacheAvailable();
+ if (expired) {
+ return;
+ }
synchronized (this.changeLock) {
// Remove the attribute locally
super.removeAttribute(name, true);
@@ -325,7 +328,7 @@ public class DeltaSession extends StandardSession
setExpired(true);
// Do expire processing
- expire();
+ super.expire(true);
// Update statistics
if (manager != null) {
@@ -333,6 +336,15 @@ public class DeltaSession extends StandardSession
}
}
+ @Override
+ public void expire(boolean notify) {
+ if (notify) {
+ getOperatingRegion().destroy(this.getId(), this);
+ } else {
+ super.expire(false);
+ }
+ }
+
public void setMaxInactiveInterval(int interval) {
super.setMaxInactiveInterval(interval);
}
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/CommandServlet.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/CommandServlet.java b/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/CommandServlet.java
index 3fede62..a04194b 100644
--- a/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/CommandServlet.java
+++ b/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/CommandServlet.java
@@ -60,6 +60,10 @@ public class CommandServlet extends HttpServlet {
session = request.getSession();
session.setAttribute(param, value);
break;
+ case SET_MAX_INACTIVE:
+ session = request.getSession();
+ session.setMaxInactiveInterval(Integer.valueOf(value));
+ break;
case GET:
session = request.getSession();
String val = (String) session.getAttribute(param);
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/QueryCommand.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/QueryCommand.java b/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/QueryCommand.java
index 2b89e68..622866e 100644
--- a/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/QueryCommand.java
+++ b/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/QueryCommand.java
@@ -21,6 +21,8 @@ public enum QueryCommand {
SET,
+ SET_MAX_INACTIVE,
+
GET,
INVALIDATE,
http://git-wip-us.apache.org/repos/asf/geode/blob/45dc6744/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/TestSessionsBase.java
----------------------------------------------------------------------
diff --git a/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/TestSessionsBase.java b/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/TestSessionsBase.java
index d7674dd..a6bec6c 100644
--- a/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/TestSessionsBase.java
+++ b/extensions/geode-modules/src/test/java/org/apache/geode/modules/session/TestSessionsBase.java
@@ -267,6 +267,40 @@ public abstract class TestSessionsBase {
}
/**
+ * Test expiration of a session by the tomcat container, rather than gemfire expiration
+ */
+ @Test
+ public void testSessionExpirationByContainer() throws Exception {
+
+ String key = "value_testSessionExpiration1";
+ String value = "Foo";
+
+ WebConversation wc = new WebConversation();
+ WebRequest req = new GetMethodWebRequest(String.format("http://localhost:%d/test", port));
+
+ // Set an attribute
+ req.setParameter("cmd", QueryCommand.SET.name());
+ req.setParameter("param", key);
+ req.setParameter("value", value);
+ WebResponse response = wc.getResponse(req);
+
+ // Set the session timeout of this one session.
+ req.setParameter("cmd", QueryCommand.SET_MAX_INACTIVE.name());
+ req.setParameter("value", "1");
+ response = wc.getResponse(req);
+
+ // Wait until the session should expire
+ Thread.sleep(2000);
+
+ // Do a request, which should cause the session to be expired
+ req.setParameter("cmd", QueryCommand.GET.name());
+ req.setParameter("param", key);
+ response = wc.getResponse(req);
+
+ assertEquals("", response.getText());
+ }
+
+ /**
* Test that removing a session attribute also removes it from the region
*/
@Test