cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dk...@apache.org
Subject git commit: [CXF-5802] Fix a condition introduced with the removeCache call where if two proxies are using the same cache, closing one (or having it GC'd) would cause the cache to become invalid.
Date Mon, 30 Jun 2014 16:44:20 GMT
Repository: cxf
Updated Branches:
  refs/heads/master d607b8cd8 -> ec4d58227


[CXF-5802] Fix a condition introduced with the removeCache call where if two proxies are using
the same cache, closing one (or having it GC'd) would cause the cache to become invalid.


Project: http://git-wip-us.apache.org/repos/asf/cxf/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/ec4d5822
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/ec4d5822
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/ec4d5822

Branch: refs/heads/master
Commit: ec4d58227e5999ba668fd23e79f677dccd73bc94
Parents: d607b8c
Author: Daniel Kulp <dkulp@apache.org>
Authored: Mon Jun 30 12:29:00 2014 -0400
Committer: Daniel Kulp <dkulp@apache.org>
Committed: Mon Jun 30 12:29:00 2014 -0400

----------------------------------------------------------------------
 .../security/tokenstore/EHCacheTokenStore.java  | 42 ++++++++++++++++++--
 .../cxf/systest/ws/cache/CachingTest.java       | 40 +++++++++++--------
 2 files changed, 62 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf/blob/ec4d5822/rt/ws/security/src/main/java/org/apache/cxf/ws/security/tokenstore/EHCacheTokenStore.java
----------------------------------------------------------------------
diff --git a/rt/ws/security/src/main/java/org/apache/cxf/ws/security/tokenstore/EHCacheTokenStore.java
b/rt/ws/security/src/main/java/org/apache/cxf/ws/security/tokenstore/EHCacheTokenStore.java
index af274b8..59f9e7a 100644
--- a/rt/ws/security/src/main/java/org/apache/cxf/ws/security/tokenstore/EHCacheTokenStore.java
+++ b/rt/ws/security/src/main/java/org/apache/cxf/ws/security/tokenstore/EHCacheTokenStore.java
@@ -22,11 +22,13 @@ package org.apache.cxf.ws.security.tokenstore;
 import java.io.Closeable;
 import java.net.URL;
 import java.util.Collection;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import net.sf.ehcache.Cache;
 import net.sf.ehcache.CacheManager;
 import net.sf.ehcache.Ehcache;
 import net.sf.ehcache.Element;
+import net.sf.ehcache.Status;
 import net.sf.ehcache.config.CacheConfiguration;
 
 import org.apache.cxf.Bus;
@@ -41,7 +43,6 @@ import org.apache.wss4j.common.cache.EHCacheManagerHolder;
  * and the max TTL is 12 hours.
  */
 public class EHCacheTokenStore implements TokenStore, Closeable, BusLifeCycleListener {
-
     public static final long DEFAULT_TTL = 3600L;
     public static final long MAX_TTL = DEFAULT_TTL * 12L;
     
@@ -61,13 +62,34 @@ public class EHCacheTokenStore implements TokenStore, Closeable, BusLifeCycleLis
         CacheConfiguration cc = EHCacheManagerHolder.getCacheConfiguration(key, cacheManager)
             .overflowToDisk(false); //tokens not writable
         
-        Ehcache newCache = new Cache(cc);
+        Cache newCache = new RefCountCache(cc);
         cache = cacheManager.addCacheIfAbsent(newCache);
+        synchronized (cache) {
+            if (cache.getStatus() != Status.STATUS_ALIVE) {
+                cache = cacheManager.addCacheIfAbsent(newCache);
+            }
+            if (cache instanceof RefCountCache) {
+                ((RefCountCache)cache).incrementAndGet();
+            }
+        }
         
         // Set the TimeToLive value from the CacheConfiguration
         ttl = cc.getTimeToLiveSeconds();
     }
     
+    private static class RefCountCache extends Cache {
+        AtomicInteger count = new AtomicInteger();
+        public RefCountCache(CacheConfiguration cc) {
+            super(cc);
+        }
+        public int incrementAndGet() {
+            return count.incrementAndGet();
+        }
+        public int decrementAndGet() {
+            return count.decrementAndGet();
+        }
+    }
+    
     /**
      * Set a new (default) TTL value in seconds
      * @param newTtl a new (default) TTL value in seconds
@@ -93,17 +115,23 @@ public class EHCacheTokenStore implements TokenStore, Closeable, BusLifeCycleLis
     }
     
     public void remove(String identifier) {
-        if (!StringUtils.isEmpty(identifier) && cache.isKeyInCache(identifier)) {
+        if (cache != null && !StringUtils.isEmpty(identifier) && cache.isKeyInCache(identifier))
{
             cache.remove(identifier);
         }
     }
 
     @SuppressWarnings("unchecked")
     public Collection<String> getTokenIdentifiers() {
+        if (cache == null) {
+            return null;
+        }
         return cache.getKeysWithExpiryCheck();
     }
     
     public SecurityToken getToken(String identifier) {
+        if (cache == null) {
+            return null;
+        }
         Element element = cache.get(identifier);
         if (element != null && !cache.isExpired(element)) {
             return (SecurityToken)element.getObjectValue();
@@ -124,7 +152,13 @@ public class EHCacheTokenStore implements TokenStore, Closeable, BusLifeCycleLis
         if (cacheManager != null) {
             // this step is especially important for global shared cache manager
             if (cache != null) {
-                cacheManager.removeCache(cache.getName());
+                synchronized (cache) {
+                    if (cache instanceof RefCountCache) {
+                        if (((RefCountCache)cache).decrementAndGet() == 0) {
+                            cacheManager.removeCache(cache.getName());
+                        }
+                    }
+                }                
             }
             
             EHCacheManagerHolder.releaseCacheManger(cacheManager);

http://git-wip-us.apache.org/repos/asf/cxf/blob/ec4d5822/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/cache/CachingTest.java
----------------------------------------------------------------------
diff --git a/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/cache/CachingTest.java
b/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/cache/CachingTest.java
index 72fa9e3..6dc9518 100644
--- a/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/cache/CachingTest.java
+++ b/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/cache/CachingTest.java
@@ -113,28 +113,35 @@ public class CachingTest extends AbstractBusClientServerTestBase {
             );
         assertNotNull(tokenStore);
         // We expect two tokens as the identifier + SHA-1 are cached
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 2);
+        assertEquals(2, tokenStore.getTokenIdentifiers().size());
+        
         
         // Second invocation
-        port = service.getPort(portQName, DoubleItPortType.class);
-        updateAddressPort(port, test.getPort());
+        DoubleItPortType port2 = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port2, test.getPort());
         
         if (test.isStreaming()) {
-            SecurityTestUtil.enableStreaming(port);
+            SecurityTestUtil.enableStreaming(port2);
         }
         
-        port.doubleIt(35);
+        port2.doubleIt(35);
 
-        client = ClientProxy.getClient(port);
+        client = ClientProxy.getClient(port2);
         tokenStore = 
             (TokenStore)client.getEndpoint().getEndpointInfo().getProperty(
                 SecurityConstants.TOKEN_STORE_CACHE_INSTANCE
             );
+
         assertNotNull(tokenStore);
         // There should now be 4 tokens as both proxies share the same TokenStore
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 4);
+        assertEquals(4, tokenStore.getTokenIdentifiers().size());
         
         ((java.io.Closeable)port).close();
+        //port2 is still holding onto the cache, thus, this should still be 4
+        assertEquals(4, tokenStore.getTokenIdentifiers().size());       
+        ((java.io.Closeable)port2).close();
+        //port2 is now closed, this should be null
+        assertNull(tokenStore.getTokenIdentifiers());       
         bus.shutdown(true);
     }
     
@@ -177,35 +184,36 @@ public class CachingTest extends AbstractBusClientServerTestBase {
             );
         assertNotNull(tokenStore);
         // We expect two tokens as the identifier + SHA-1 are cached
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 2);
+        assertEquals(2, tokenStore.getTokenIdentifiers().size());
         
         // Second invocation
-        port = service.getPort(portQName, DoubleItPortType.class);
-        updateAddressPort(port, test.getPort());
+        DoubleItPortType port2 = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port2, test.getPort());
         
-        ((BindingProvider)port).getRequestContext().put(
+        ((BindingProvider)port2).getRequestContext().put(
             SecurityConstants.CACHE_IDENTIFIER, "proxy2"
         );
-        ((BindingProvider)port).getRequestContext().put(
+        ((BindingProvider)port2).getRequestContext().put(
             SecurityConstants.CACHE_CONFIG_FILE, "per-proxy-cache.xml"
         );
         
         if (test.isStreaming()) {
-            SecurityTestUtil.enableStreaming(port);
+            SecurityTestUtil.enableStreaming(port2);
         }
         
-        port.doubleIt(35);
+        port2.doubleIt(35);
 
-        client = ClientProxy.getClient(port);
+        client = ClientProxy.getClient(port2);
         tokenStore = 
             (TokenStore)client.getEndpoint().getEndpointInfo().getProperty(
                 SecurityConstants.TOKEN_STORE_CACHE_INSTANCE
             );
         assertNotNull(tokenStore);
         // We expect two tokens as the identifier + SHA-1 are cached
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 2);
+        assertEquals(2, tokenStore.getTokenIdentifiers().size());
         
         ((java.io.Closeable)port).close();
+        ((java.io.Closeable)port2).close();
         bus.shutdown(true);
     }
     


Mime
View raw message