Michael Blow has submitted this change and it was merged. Change subject: [ASTERIXDB-2091][HYR][CLUS] Fix unsafe concurrent access to config on NC registration ......................................................................
[ASTERIXDB-2091][HYR][CLUS] Fix unsafe concurrent access to config on NC registration Change-Id: I399d26d128794a0a52eff419f59ca28d64b17375 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2007 Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Murtadha Hubail <mhub...@apache.org> --- M hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java 1 file changed, 9 insertions(+), 5 deletions(-) Approvals: Anon. E. Moose #1000171: Jenkins: Verified; No violations found; ; Verified Murtadha Hubail: Looks good to me, approved diff --git a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java index 2e04e13..d13b5e6 100644 --- a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java +++ b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java @@ -71,7 +71,8 @@ private CompositeMap<IOption, Object> configurationMap = new CompositeMap<>(definedMap, defaultMap, new NoOpMapMutator()); private EnumMap<Section, Map<String, IOption>> sectionMap = new EnumMap<>(Section.class); - private TreeMap<String, Map<IOption, Object>> nodeSpecificMap = new TreeMap<>(); + @SuppressWarnings("squid:S1948") // TreeMap is serializable, and therefore so is its synchronized map + private Map<String, Map<IOption, Object>> nodeSpecificMap = Collections.synchronizedMap(new TreeMap<>()); private transient ArrayListValuedHashMap<IOption, IConfigSetter> optionSetters = new ArrayListValuedHashMap<>(); private final String[] args; private ConfigManagerApplicationConfig appConfig = new ConfigManagerApplicationConfig(this); @@ -457,10 +458,13 @@ } for (Map.Entry<String, Map<IOption, Object>> nodeMapEntry : nodeSpecificMap.entrySet()) { String section = Section.NC.sectionName() + "/" + nodeMapEntry.getKey(); - for (Map.Entry<IOption, Object> entry : nodeMapEntry.getValue().entrySet()) { - if (entry.getValue() != null) { - final IOption option = entry.getKey(); - ini.add(section, option.ini(), option.type().serializeToIni(entry.getValue())); + final Map<IOption, Object> nodeValueMap = nodeMapEntry.getValue(); + synchronized (nodeValueMap) { + for (Map.Entry<IOption, Object> entry : nodeValueMap.entrySet()) { + if (entry.getValue() != null) { + final IOption option = entry.getKey(); + ini.add(section, option.ini(), option.type().serializeToIni(entry.getValue())); + } } } } -- To view, visit https://asterix-gerrit.ics.uci.edu/2007 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I399d26d128794a0a52eff419f59ca28d64b17375 Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org>
