Improve thread safety for HashMap instances across the codebase #48
Description
Overview
This issue addresses potential thread safety concerns with HashMap instances throughout the codebase. Since the application operates in a multi-threaded environment (JDA events can be fired from different threads), unsynchronized access to HashMaps can lead to data corruption, race conditions, or ConcurrentModificationExceptions.
Locations requiring synchronization
ConfigManager.Config class
private final Map<String, String> config = new HashMap<>()private transient final Map<String, Type> configTypes = new HashMap<>()- Need to synchronize methods or replace with ConcurrentHashMap
CacheManager class
public static final HashMap<String, List<MemberStructure>> guildMemberCache = new HashMap<>()public static final HashMap<String, List<GuildStructure>> mutualGuildsCache = new HashMap<>()public static final HashMap<String, List<MessageStructure>> channelMessageCache = new HashMap<>()public static final HashMap<String, List<MessageStructure>> userMessageCache = new HashMap<>()public static final HashMap<String, User> userCache = new HashMap<>()- These static caches need thread-safe access as they're modified in JDA event listeners
CommandManager class
private static final HashMap<CommandInfo, Method> commands = new HashMap<>()private static final HashMap<String, List<Map.Entry<CommandInfo, Method>>> subcommands = new HashMap<>()private static final HashMap<OptionType, Class<?>> typeMap = new HashMap<>(){{}}- These static maps need thread-safe access if they're modified after initialization
MenuManager class
public static final Map<String, IMenu> menuRegistry = new HashMap<>()- This registry is potentially accessed from multiple threads
CommandInfo class
private static final HashMap<Class<?>, OptionType> optionTypeParams = new HashMap<>() {{}}- Thread safety may be needed if modified after initialization
Config class
- Extends HashMap directly, which isn't thread-safe
- Consider redesigning to use composition with a ConcurrentHashMap instead of inheritance
StringUtilities and JsonUtils
- Various HashMap usages, but likely only local to methods (safer)
- Review to ensure none are stored in static fields or accessed across threads
Recommended solutions
- Replace HashMap with ConcurrentHashMap where concurrent modification is possible
- Add synchronization to methods that modify HashMap instances
- For static initializers with {{}} syntax, ensure they're only modified during initialization
- Use thread-safe collections like Collections.synchronizedMap() where appropriate
Impact
Implementing these changes will improve application stability and prevent hard-to-debug concurrency issues.