Use ConcurrentHashMap for CloudExpansionManager cache - #1246

Pull Request

Type

• Internal change (Doesn't affect end-user).
• External change (Does affect end-user).
• Wiki (Changes towards the [Wiki]).
• Other: __________

Description

The cache field in CloudExpansionManager is a plain HashMap that is read from multiple public methods (getCloudExpansions(), findCloudExpansionByName(), getCloudExpansionsInstalled(), getCloudExpansionsByAuthor(), getCloudExpansionAuthors(), isEmpty()) and written to from fetch() (via the async-then-main-thread callback) and clean().

Since these public read methods can be called from any thread while the async fetch() completes on the main thread, this creates a potential ConcurrentModificationException risk when iterating the map (e.g. via ImmutableMap.copyOf(cache) or streams over cache.values()).

The sister class LocalExpansionManager already uses ConcurrentHashMap for its expansions map, and the local values map inside fetch() in this same class already uses ConcurrentHashMap with an explicit comment:

// a defence tactic! use ConcurrentHashMap instead of normal HashMap
Map<String, CloudExpansion> values = new ConcurrentHashMap<>();