[HDFS-14610] HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.

• Type: Bug
• Status: Resolved
• Priority: Critical
• Resolution: Fixed
• Fix Version/s: 3.3.0, 3.1.4, 3.2.2
• Labels: fix-provided patch-available
• Hadoop Flags: Reviewed
• Flags: Patch
• Assignee: Paul Ward
• Reporter: Paul Ward
• Created: 26/Jun/19 00:00
• Updated: 02/Oct/19 21:38
• Resolved: 01/Jul/19 21:03

Description

I submitted a CR for this issue at:

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:

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();
}

 

The CR I inlined above protected the above instance (line 455 ) with synchronization
like in line 484 and in all other occurrences.