You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@turbine.apache.org by tv...@apache.org on 2016/01/19 11:11:24 UTC
svn commit: r1725458 - in
/turbine/core/trunk/src/java/org/apache/turbine/services:
BaseServiceBroker.java assemblerbroker/util/java/JavaBaseFactory.java
Author: tv
Date: Tue Jan 19 10:11:23 2016
New Revision: 1725458
URL: http://svn.apache.org/viewvc?rev=1725458&view=rev
Log:
Address concurrency issues reported by Findbugs
Modified:
turbine/core/trunk/src/java/org/apache/turbine/services/BaseServiceBroker.java
turbine/core/trunk/src/java/org/apache/turbine/services/assemblerbroker/util/java/JavaBaseFactory.java
Modified: turbine/core/trunk/src/java/org/apache/turbine/services/BaseServiceBroker.java
URL: http://svn.apache.org/viewvc/turbine/core/trunk/src/java/org/apache/turbine/services/BaseServiceBroker.java?rev=1725458&r1=1725457&r2=1725458&view=diff
==============================================================================
--- turbine/core/trunk/src/java/org/apache/turbine/services/BaseServiceBroker.java (original)
+++ turbine/core/trunk/src/java/org/apache/turbine/services/BaseServiceBroker.java Tue Jan 19 10:11:23 2016
@@ -21,8 +21,6 @@ package org.apache.turbine.services;
import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.Hashtable;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
@@ -119,7 +117,7 @@ public abstract class BaseServiceBroker
/**
* mapping from service names to instances of TurbineServiceProviders
*/
- private final Hashtable<String, Service> serviceProviderInstanceMap = new Hashtable<String, Service>();
+ private final ConcurrentHashMap<String, Service> serviceProviderInstanceMap = new ConcurrentHashMap<String, Service>();
/**
* Default constructor, protected as to only be usable by subclasses.
@@ -643,7 +641,11 @@ public abstract class BaseServiceBroker
// service provider - if so then remember it
if (service instanceof TurbineServiceProvider)
{
- this.serviceProviderInstanceMap.put(name,service);
+ Service _service = this.serviceProviderInstanceMap.putIfAbsent(name,service);
+ if (_service != null)
+ {
+ service = _service;
+ }
}
}
// those two errors must be passed to the VM
@@ -672,7 +674,11 @@ public abstract class BaseServiceBroker
}
service.setServiceBroker(this);
service.setName(name);
- services.put(name, service);
+ Service _service = services.putIfAbsent(name, service);
+ if (_service != null) // Unlikely
+ {
+ service = _service;
+ }
}
}
finally
@@ -740,14 +746,11 @@ public abstract class BaseServiceBroker
*/
protected boolean isNonLocalService(String name)
{
- String serviceName = null;
TurbineServiceProvider turbineServiceProvider = null;
- Enumeration<String> list = this.serviceProviderInstanceMap.keys();
- while (list.hasMoreElements())
+ for (Map.Entry<String, Service> entry : this.serviceProviderInstanceMap.entrySet())
{
- serviceName = list.nextElement();
- turbineServiceProvider = (TurbineServiceProvider) this.getService(serviceName);
+ turbineServiceProvider = (TurbineServiceProvider) this.getService(entry.getKey());
if (turbineServiceProvider.exists(name))
{
@@ -768,14 +771,11 @@ public abstract class BaseServiceBroker
protected Object getNonLocalService(String name)
throws InstantiationException
{
- String serviceName = null;
TurbineServiceProvider turbineServiceProvider = null;
- Enumeration<String> list = this.serviceProviderInstanceMap.keys();
- while (list.hasMoreElements())
+ for (Map.Entry<String, Service> entry : this.serviceProviderInstanceMap.entrySet())
{
- serviceName = list.nextElement();
- turbineServiceProvider = (TurbineServiceProvider) this.getService(serviceName);
+ turbineServiceProvider = (TurbineServiceProvider) this.getService(entry.getKey());
if (turbineServiceProvider.exists(name))
{
Modified: turbine/core/trunk/src/java/org/apache/turbine/services/assemblerbroker/util/java/JavaBaseFactory.java
URL: http://svn.apache.org/viewvc/turbine/core/trunk/src/java/org/apache/turbine/services/assemblerbroker/util/java/JavaBaseFactory.java?rev=1725458&r1=1725457&r2=1725458&view=diff
==============================================================================
--- turbine/core/trunk/src/java/org/apache/turbine/services/assemblerbroker/util/java/JavaBaseFactory.java (original)
+++ turbine/core/trunk/src/java/org/apache/turbine/services/assemblerbroker/util/java/JavaBaseFactory.java Tue Jan 19 10:11:23 2016
@@ -22,7 +22,6 @@ package org.apache.turbine.services.asse
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.locks.ReentrantLock;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
@@ -56,11 +55,6 @@ public abstract class JavaBaseFactory<T
private final ConcurrentHashMap<String, Class<T>> classCache = new ConcurrentHashMap<String, Class<T>>();
/**
- * Lock for cache update
- */
- private final ReentrantLock lock = new ReentrantLock();
-
- /**
* Get an Assembler.
*
* @param packageName java package name
@@ -96,22 +90,11 @@ public abstract class JavaBaseFactory<T
Class<T> servClass = classCache.get(className);
if (servClass == null)
{
- lock.lock();
-
- try
- {
- // Double check
- servClass = classCache.get(className);
-
- if (servClass == null)
- {
- servClass = (Class<T>) Class.forName(className);
- classCache.put(className, servClass);
- }
- }
- finally
+ servClass = (Class<T>) Class.forName(className);
+ Class<T> _servClass = classCache.putIfAbsent(className, servClass);
+ if (_servClass != null)
{
- lock.unlock();
+ servClass = _servClass;
}
}
assembler = servClass.newInstance();