From 16b34824673f5a50d464727b8fad98470e5e984a Mon Sep 17 00:00:00 2001 From: Jing Zhao Date: Thu, 6 Nov 2014 16:07:50 -0800 Subject: [PATCH] HADOOP-11274. ConcurrentModificationException in Configuration Copy Constructor. Contributed by Junping Du. --- .../hadoop-common/CHANGES.txt | 3 + .../org/apache/hadoop/conf/Configuration.java | 81 ++++++++++--------- 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 8587f1297e2..679b5bb4ebf 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -910,6 +910,9 @@ Release 2.6.0 - UNRELEASED HADOOP-11253. Hadoop streaming test TestStreamXmlMultipleRecords fails on Windows. (Varun Vasudev via wheat9) + HADOOP-11274. ConcurrentModificationException in Configuration Copy Constructor. + (Junping Du via jing9) + BREAKDOWN OF HDFS-6134 AND HADOOP-10150 SUBTASKS AND RELATED JIRAS HADOOP-10734. Implement high-performance secure random number sources. diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index a3fae19b5b1..16d549901b2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -690,26 +690,26 @@ public Configuration(boolean loadDefaults) { */ @SuppressWarnings("unchecked") public Configuration(Configuration other) { - this.resources = (ArrayList) other.resources.clone(); - synchronized(other) { - if (other.properties != null) { - this.properties = (Properties)other.properties.clone(); - } + synchronized(other) { + this.resources = (ArrayList) other.resources.clone(); + if (other.properties != null) { + this.properties = (Properties)other.properties.clone(); + } - if (other.overlay!=null) { - this.overlay = (Properties)other.overlay.clone(); - } + if (other.overlay!=null) { + this.overlay = (Properties)other.overlay.clone(); + } - this.updatingResource = new HashMap(other.updatingResource); - this.finalParameters = new HashSet(other.finalParameters); - } - + this.updatingResource = new HashMap(other.updatingResource); + this.finalParameters = new HashSet(other.finalParameters); + + this.classLoader = other.classLoader; + this.loadDefaults = other.loadDefaults; + setQuietMode(other.getQuietMode()); + } synchronized(Configuration.class) { REGISTRY.put(this, null); } - this.classLoader = other.classLoader; - this.loadDefaults = other.loadDefaults; - setQuietMode(other.getQuietMode()); } /** @@ -1025,26 +1025,28 @@ public void set(String name, String value, String source) { getProps().setProperty(name, value); String newSource = (source == null ? "programmatically" : source); - if (!isDeprecated(name)) { - updatingResource.put(name, new String[] {newSource}); - String[] altNames = getAlternativeNames(name); - if(altNames != null) { - for(String n: altNames) { - if(!n.equals(name)) { - getOverlay().setProperty(n, value); - getProps().setProperty(n, value); - updatingResource.put(n, new String[] {newSource}); + synchronized (this) { + if (!isDeprecated(name)) { + updatingResource.put(name, new String[] {newSource}); + String[] altNames = getAlternativeNames(name); + if(altNames != null) { + for(String n: altNames) { + if(!n.equals(name)) { + getOverlay().setProperty(n, value); + getProps().setProperty(n, value); + updatingResource.put(n, new String[] {newSource}); + } } } } - } - else { - String[] names = handleDeprecation(deprecationContext.get(), name); - String altSource = "because " + name + " is deprecated"; - for(String n : names) { - getOverlay().setProperty(n, value); - getProps().setProperty(n, value); - updatingResource.put(n, new String[] {altSource}); + else { + String[] names = handleDeprecation(deprecationContext.get(), name); + String altSource = "because " + name + " is deprecated"; + for(String n : names) { + getOverlay().setProperty(n, value); + getProps().setProperty(n, value); + updatingResource.put(n, new String[] {altSource}); + } } } } @@ -2277,7 +2279,7 @@ public Reader getConfResourceAsReader(String name) { * * @return final parameter set. */ - public Set getFinalParameters() { + public synchronized Set getFinalParameters() { return new HashSet(finalParameters); } @@ -2540,14 +2542,18 @@ private void loadProperty(Properties properties, String name, String attr, if (value != null) { if (!finalParameters.contains(attr)) { properties.setProperty(attr, value); - updatingResource.put(attr, source); + synchronized(this) { + updatingResource.put(attr, source); + } } else if (!value.equals(properties.getProperty(attr))) { LOG.warn(name+":an attempt to override final parameter: "+attr +"; Ignoring."); } } if (finalParameter) { - finalParameters.add(attr); + synchronized(this) { + finalParameters.add(attr); + } } } @@ -2741,7 +2747,7 @@ public static void main(String[] args) throws Exception { } @Override - public void readFields(DataInput in) throws IOException { + public synchronized void readFields(DataInput in) throws IOException { clear(); int size = WritableUtils.readVInt(in); for(int i=0; i < size; ++i) { @@ -2753,9 +2759,8 @@ public void readFields(DataInput in) throws IOException { } } - //@Override @Override - public void write(DataOutput out) throws IOException { + public synchronized void write(DataOutput out) throws IOException { Properties props = getProps(); WritableUtils.writeVInt(out, props.size()); for(Map.Entry item: props.entrySet()) {