cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dk...@apache.org
Subject [2/4] 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 17:28:16 GMT
[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.

Conflicts:
	rt/ws/security/src/main/java/org/apache/cxf/ws/security/tokenstore/EHCacheTokenStore.java
	systests/ws-security/src/test/java/org/apache/cxf/systest/ws/cache/CachingTest.java


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

Branch: refs/heads/2.7.x-fixes
Commit: 353771670f917714c8370a5bf45c298649622c69
Parents: a25efba
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 13:17:06 2014 -0400

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


http://git-wip-us.apache.org/repos/asf/cxf/blob/35377167/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 3ee6c84..4270934 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
@@ -26,11 +26,13 @@ import java.util.Collection;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
+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;
@@ -44,7 +46,6 @@ import org.apache.cxf.ws.security.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;
     
@@ -64,13 +65,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
@@ -114,13 +136,16 @@ 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();
     }
     
@@ -138,6 +163,9 @@ public class EHCacheTokenStore implements TokenStore, Closeable, BusLifeCycleLis
     }
     
     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();
@@ -179,7 +207,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/35377167/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 c324e07..d3c73e8 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
@@ -90,24 +90,32 @@ public class CachingTest extends AbstractBusClientServerTestBase {
             );
         assertNotNull(tokenStore);
         // We expect 1 token
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 1);
+        assertEquals(1, tokenStore.getTokenIdentifiers().size());
+        
+        
         
         // Second invocation
-        port = service.getPort(portQName, DoubleItPortType.class);
-        updateAddressPort(port, PORT);
+        DoubleItPortType port2 = service.getPort(portQName, DoubleItPortType.class);
+        updateAddressPort(port2, test.getPort());
         
-        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 2 tokens as both proxies share the same TokenStore
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 2);
+        assertEquals(2, 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);
     }
     
@@ -146,31 +154,33 @@ public class CachingTest extends AbstractBusClientServerTestBase {
             );
         assertNotNull(tokenStore);
         // We expect 1 token
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 1);
+        assertEquals(1, tokenStore.getTokenIdentifiers().size());
+        
         
         // Second invocation
-        port = service.getPort(portQName, DoubleItPortType.class);
-        updateAddressPort(port, PORT);
+        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, "client/per-proxy-cache.xml"
         );
         
-        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 1 token
-        assertEquals(tokenStore.getTokenIdentifiers().size(), 1);
+        assertEquals(1, tokenStore.getTokenIdentifiers().size());
         
         ((java.io.Closeable)port).close();
+        ((java.io.Closeable)port2).close();
         bus.shutdown(true);
     }
     


Mime
View raw message