Skip to content
Snippets Groups Projects
Commit ded9cead authored by Grégory Mantelet's avatar Grégory Mantelet
Browse files

[UWS] Fix recurrent ConcurrentModificationException during service overload. 

This error occurred generally during the backup process while trying to
backup the job list of a specific user. If several of his jobs were running
and changing state during the backup process, this
ConcurrentModificationException was thrown. This generally happens when the same
user submits a lot of shorts jobs in the same time.

This exception was due to a non thread-safe usage of
UWSParameters.additionalParams. To fix this issue, instead of creating it as a
normal HashMap, it is now created as a ConcurrentHashMap.

The same modification has also been applied to UWSParameters.params. In addition
of the replacement of HashMap into ConcurrentHashMap, all `synchronized` blocks
have been removed....there should not be needed any more.
parent 1f4bc6b1
No related branches found
No related tags found
No related merge requests found
Show whitespace changes
Inline Side-by-side
src/uws/job/parameters/UWSParameters.java
+ 198
− 211
View file @ ded9cead
......@@ -16,7 +16,7 @@ package uws.job.parameters;
* You should have received a copy of the GNU Lesser General Public License
* along with UWSLibrary. If not, see <http://www.gnu.org/licenses/>.
*
* Copyright 2012-2017 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
* Copyright 2012-2018 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
* Astronomisches Rechen Institut (ARI)
*/
......@@ -32,6 +32,7 @@ import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.servlet.http.HttpServletRequest;
......@@ -116,7 +117,7 @@ import uws.service.request.UploadFile;
* <p><i><u>note 2:</u> If several values have been submitted for the same UWS standard parameter, just the last occurrence is taken into account.</i></p>
*
* @author Gr&eacute;gory Mantelet (CDS;ARI)
* @version 4.2 (09/2017)
* @version 4.4 (07/2018)
*/
public class UWSParameters implements Iterable<Entry<String, Object>> {
......@@ -138,7 +139,7 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
/**
* List of all extracted parameters.
*/
protected final Map<String,Object> params = new HashMap<String,Object>(10);
protected final Map<String, Object> params = new ConcurrentHashMap<String, Object>(10);
/**
* <p>List of all UWS additional parameters.</p>
......@@ -258,7 +259,8 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
try{
if (request.getAttribute(UWS.REQ_ATTRIBUTE_PARAMETERS) != null)
return (Map<String, Object>)request.getAttribute(UWS.REQ_ATTRIBUTE_PARAMETERS);
}catch(Exception e){} // 2 possible exceptions: ClassCastException and NullPointerException
}catch(Exception e){
} // 2 possible exceptions: ClassCastException and NullPointerException
/* If there is no such attribute or if it is not of the good type,
* extract only application/x-www-form-urlencoded parameters: */
......@@ -392,12 +394,11 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
/**
* <p>Set the default value to all missing parameters (ONLY for those who have a controller -&gt; see {@link InputParamController}).</p>
*
* <p><i><u>note:</u> This method is thread safe (synchronized on the list of parameters) !</i></p>
* <p><i><u>note:</u> This method is thread safe!</i></p>
*/
public final void init(){
Iterator<Entry<String, InputParamController>> itControllers = getControllers();
if (itControllers != null){
synchronized(params){
Entry<String, InputParamController> entry;
while(itControllers.hasNext()){
entry = itControllers.next();
......@@ -406,13 +407,12 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
}
}
}
}
/**
* <p>Updates this list of UWS parameters with the given ones. No check is done on the given parameters
* (since they come from an instance of {@link UWSParameters}, they are supposed to be correct)</p>
*
* <p><i><u>note:</u> This method is thread safe (synchronized on the list of parameters) !</i></p>
* <p><i><u>note:</u> This method is thread safe!</i></p>
*
* @param newParams The parameters to update.
*
......@@ -422,7 +422,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
*/
public String[] update(final UWSParameters newParams) throws UWSException{
if (newParams != null && !newParams.params.isEmpty()){
synchronized(params){
additionalParams = null;
files = null;
String[] updated = new String[newParams.params.size()];
......@@ -445,7 +444,8 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
if (oldValue != null && oldValue instanceof UploadFile){
try{
((UploadFile)oldValue).deleteFile();
}catch(IOException ioe){}
}catch(IOException ioe){
}
}
// Perform the replacement:
params.put(entry.getKey(), entry.getValue());
......@@ -457,7 +457,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
updated[i++] = entry.getKey();
}
return updated;
}
}else
return new String[0];
}
......@@ -497,7 +496,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
public final Iterator<UploadFile> getFiles(){
if (files == null){
files = new ArrayList<UploadFile>(3);
synchronized(params){
for(Object v : params.values()){
if (v == null)
continue;
......@@ -511,7 +509,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
}
}
}
}
return files.iterator();
}
......@@ -519,7 +516,7 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
* <p>Sets the given value to the specified parameter.
* But if the given value is <i>null</i>, the specified parameter is merely removed.</p>
*
* <p><i><u>note 1:</u> This method is thread safe (synchronized on the list of parameters) !</i></p>
* <p><i><u>note 1:</u> This method is thread safe!</i></p>
* <p><i><u>note 2:</u> The case of the parameter name MUST BE correct EXCEPT FOR the standard UWS parameters (i.e. runId, executionDuration, destructionTime).</i></p>
* <p><i><u>note 3:</u> If the specified parameter is a read-only UWS parameter (i.e. jobId, startTime, endTime, results, errorSummary, quote), this function does nothing and merely returns NULL.</i></p>
* <p><i><u>note 4:</u> A value equals to NULL, means that the specified parameter must be removed.</i></p>
......@@ -547,7 +544,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
if (normalizedName == null)
return null;
synchronized(params){
additionalParams = null;
files = null;
......@@ -580,14 +576,14 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
if (oldValue != null && oldValue instanceof UploadFile){
try{
((UploadFile)oldValue).deleteFile();
}catch(IOException ioe){}
}catch(IOException ioe){
}
}
// Set the new value:
return params.put(normalizedName, value);
}
}
}
/**
* Removes the value of the given input parameter name.
......@@ -606,7 +602,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
if (normalizedName == null)
return null;
synchronized(params){
additionalParams = null;
files = null;
// Remove the file:
......@@ -615,12 +610,12 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
if (removed != null && removed instanceof UploadFile){
try{
((UploadFile)removed).deleteFile();
}catch(IOException ioe){}
}catch(IOException ioe){
}
}
// Return the value of the removed parameter:
return removed;
}
}
/**
* Normalizes the given name.
......@@ -669,13 +664,11 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
* @return The given phase.
*/
public final String getInputPhase(){
synchronized(params){
if (hasInputPhase())
return params.remove(UWSJob.PARAM_PHASE).toString();
else
return null;
}
}
/**
* Gets the value of the parameter {@link UWSJob#PARAM_RUN_ID runId}.
......@@ -697,9 +690,7 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
else if (value instanceof String){
try{
Long duration = Long.parseLong((String)value);
synchronized(params){
params.put(UWSJob.PARAM_EXECUTION_DURATION, duration);
}
return duration;
}catch(NumberFormatException nfe){
;
......@@ -721,9 +712,7 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
else if (value instanceof String){
try{
Date destruction = ISO8601Format.parseToDate((String)value);
synchronized(params){
params.put(UWSJob.PARAM_DESTRUCTION_TIME, destruction);
}
return destruction;
}catch(ParseException pe){
;
......@@ -743,8 +732,7 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
*/
public final Map<String, Object> getAdditionalParameters(){
if (additionalParams == null){
synchronized(params){
additionalParams = new HashMap<String,Object>(params.size());
additionalParams = new ConcurrentHashMap<String, Object>(params.size());
for(Entry<String, Object> entry : params.entrySet()){
boolean uwsParam = false;
for(int i = 0; !uwsParam && i < UWS_RW_PARAMETERS.length; i++)
......@@ -755,7 +743,6 @@ public class UWSParameters implements Iterable<Entry<String,Object>> {
additionalParams.put(entry.getKey(), entry.getValue());
}
}
}
return additionalParams;
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment