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 <[email protected]> Authored: Mon Jun 30 12:29:00 2014 -0400 Committer: Daniel Kulp <[email protected]> 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); }
