s1monw commented on a change in pull request #637: LUCENE-8754: Prevent 
ConcurrentModificationException in SegmentInfo
URL: https://github.com/apache/lucene-solr/pull/637#discussion_r272983458
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/index/SegmentInfo.java
 ##########
 @@ -326,7 +327,16 @@ public String getAttribute(String key) {
    * value.
    */
   public String putAttribute(String key, String value) {
-    return attributes.put(key, value);
+    HashMap<String, String> newMap = new HashMap<>(attributes);
+    String oldValue = newMap.put(key, value);
+    // we make a full copy of this to prevent concurrent modifications to this 
in the toString method
+    // this method is only called when a segment is written but the 
SegmentInfo might be exposed
+    // in running merges which can cause ConcurrentModificationExceptions if 
we modify / share
+    // the same instance. Technically that's an unsafe publication but IW 
design would require
+    // significant changes to prevent this. On the other hand, since we expose 
the map in getAttributes()
+    // it's a good design to make it unmodifiable anyway.
+    attributes = Collections.unmodifiableMap(new 
HashMap<>(Objects.requireNonNull(newMap)));
 
 Review comment:
   sure

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to