[GitHub] [hadoop] paulward24 opened a new pull request #1015: HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.

[GitBox <gi...@apache.org>]

paulward24 opened a new pull request #1015: HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
URL: https://github.com/apache/hadoop/pull/1015
 
 
   The field ```storageMap```  (a ```HashMap```)
   
   https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155
   
   is typically protected by synchronization on ```storageMap```, e.g., 
   
   https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294
   
   https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443
   
   https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484
   
   For a total of 9 locations.
   
   The reason is because ```HashMap``` is not thread safe.
   
   However, here:
   
   https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455
   
   ```
   DatanodeStorageInfo storage =
     storageMap.get(report.getStorage().getStorageID());
   ```
   
   It is not synchronized.
   
   Note that in the same method (about 30 lines below):
   
   https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484
   
   ```storageMap``` is again protected by synchronization:
   
   ```
   synchronized (storageMap) {
     storageMapSize = storageMap.size();
   }
   ```
   
   This CR protected the above instance (line 455 ) with synchronization
   like in line 484 and in all other occurrences.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org