geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zho...@apache.org
Subject [6/7] geode git commit: GEODE-2808 - Fixing lock ordering issues in DeltaSession
Date Thu, 27 Apr 2017 06:27:25 GMT
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/80a95f60
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/80a95f60
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/80a95f60

Branch: refs/heads/feature/GEM-1299
Commit: 80a95f6047ad64d165744f5ae4088b409484d50f
Parents: 5a8d03d
Author: Dan Smith <upthewaterspout@apache.org>
Authored: Fri Apr 21 11:36:24 2017 -0700
Committer: zhouxh <gzhou@pivotal.io>
Committed: Wed Apr 26 23:23:48 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/80a95f60/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/80a95f60/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/80a95f60/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/80a95f60/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/80a95f60/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/80a95f60/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/80a95f60/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


Mime
View raw message