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