Concurrent calls to singleton class method produces inconsistent results

Question

I have a singleton class that has a single method that reads all files from a directory. The configRootDir and ContentType (An Enum for type reference) are passed in. readAllConfigsFromLocalDisk method lists all files in the directory and process one by one to map file content to an expected object type according to ContentType parameter.

// Config type reference
public enum ConfigType {
    MY_TYPE, MY_OTHER_TYPE
}

// Singleton class
public class Singleton {
    private static Singleton instance;
    private Map<String, MyType> myTypeMap = new HashMap();
    private Map<String, MyOtherType> myOtherTypeMap = new HashMap();

    private Singleton() {}

    public synchronized static Singleton getSingleton() {
        if (istance == null)
            istance = new Singleton();
        return istance;
    }

    public Map<String,MyType> getMyTypeMap(String filePath, ConfigType configType, String filePattern){
        myTypeMap.clear();
        readAllConfigsFromLocalDisk(configRootDir, configType, filePattern);
        return myTypeMap;
    }

    public Map<String,MyOtherType> getMyOtherTypeMap(String filePath, ConfigType configType, String filePattern){
        myOtherTypeMap.clear();
        readAllConfigsFromLocalDisk(configRootDir, configType, filePattern);
        return myOtherTypeMap;
    }

    /**
     * Get all files in config root directory and parse one by one
     * @param configRootDir Root directory for configurations
     * @param configType Configuration type
     * @param filePattern File pattern
     */
    private void readAllConfigsFromLocalDisk(String configRootDir, ConfigType configType, String filePattern) {
        try (Stream<Path> walk = Files.walk(Paths.get(configRootDir))) {
            Pattern pattern = Pattern.compile(filePattern);
            List<Path> filePaths = getLocalFilePaths(walk, pattern);

            if (!filePaths.isEmpty()) {
                for (Path filePath : filePaths) {
                    String relativePath = filePath.toString();
                    parseConfigFile(relativePath, configType);
                }
            }
        } catch (IOException ex) {
            logger.error("Specified config root directory not found.", ex);
        }
    }

    /**
     * Read a given configuration file  from local disk and map to specified config type
     *
     * @param configFile Relative path to config file on local disk
     * @param configType Configuration type (MY_TYPE or MY_OTHER_TYPE)
     */
    private void parseConfigFile(String filePath, ConfigType configType ){
        String configContent = Files.readString(Paths.get(filePath), Charsets.UTF_8);
        
        // Parse based on config type and overwrite map
        switch (configType) {
            case MY_TYPE:
                MyTypeConf myTypeConf = Core.getMapper().readValue(configContent, MyTypeConf.class);
                List<MyType> myTypeRefs = myTypeConf.getMyTypeList();
                myTypeMap.putAll(myTypeRefs.stream().collect(Collectors.toMap(MyType::getId, Function.identity())));
            case MY_OTHER_TYPE:
                MyOtherTypeConf myOtherTypeConf = Core.getMapper().readValue(configContent, MyOtherTypeConf.class);
                List<MyOtherType> myOtherTypeRefs = myOtherTypeConf.getMyOtherTypeList();
                myOtherTypeMap.putAll(myOtherTypeRefs.stream().collect(Collectors.toMap(MyOtherType::getId, Function.identity())));
        }
    }

    /**
     * Get file paths of all matching files exist in configured streaming directory and sub folders from disk.
     *
     * @param walk    Stream of paths in config root directory.
     * @param pattern Pattern to math when discovering files.
     * @return List of Path objects for all files matching the pattern.
     */
    private List<Path> getLocalFilePaths(Stream<Path> walk, Pattern pattern) {
        return walk.filter(Files::isRegularFile).filter(p -> {
            String fileName = p.getFileName().toString();
            Matcher matcher = pattern.matcher(fileName);
            return matcher.matches();
        }).collect(Collectors.toList());
    }
}

Two public methods getMyTypeMap and getMyOtherTypeMap are called concurrently by a set of Akka actors. I get com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException when mapping the file content to Objects in some occasions.

It seems the reason is configContent was actually MyType parsable when trying to map it to MyOtherType and vise versa.

I looked at a few other places but unable to get the full picture of it. I'm trying to understand what happens when readFile is called concurrently and why it mixup file content. Can someone help me to understand this? Thanks in advance.

Answer 1

You have declared two shared variables:

private Map<String, MyType> myTypeMap = new HashMap();
private Map<String, MyOtherType> myOtherTypeMap = new HashMap();

Since HashMap is not thread-safe, the strangest things can happen when multiple threads access an instance of it concurrently (and at least one thread is modifying it).

Using a thread safe map would not fix the semantic issue, as all invocations of getMyTypeMap return the same map instance and manipulate it, so a caller can not use the returned map reliably as other threads still executing getMyTypeMap are changing it (again). The same applies to concurrent calls of getMyOtherTypeMap.

Since each method starts with a clear() invocation, it seems that sharing data between different invocations of the method is not intended, therefore, these method should not share data.

It seems, the main obstacle to you was how to reuse the code for getting different result types. Don’t use that enum type:

public class Singleton {
    /**
     * Classes are already lazily initialized, on first getSingleton() call
     */
    private static final Singleton instance = new Singleton();

    private Singleton() {}

    public static Singleton getSingleton() {
        return instance;
    }

    public Map<String, MyType> getMyTypeMap(String configRootDir){
        return readAllConfigsFromLocalDisk(configRootDir, "my-type-file-pattern",
            MyTypeConf.class, MyTypeConf::getMyTypeList, MyType::getId);
    }

    public Map<String, MyOtherType> getMyOtherTypeMap(String configRootDir){
        return readAllConfigsFromLocalDisk(configRootDir, "my-other-type-file-pattern",
            MyOtherTypeConf.class,MyOtherTypeConf::getMyOtherTypeList,MyOtherType::getId);
    }

    /**
     * Get all files in config root directory and parse one by one
     * @param configRootDir Root directory for configurations
     * @param filePattern File pattern
     * @param confType Configuration type (MyTypeConf.class or MyOtherTypeConf.class)
     * @param getList Configuration type specific list accessor method
     * @param getId Result type specific Id accessor for the map key
     */
    private <T,C> Map<String,T> readAllConfigsFromLocalDisk(
        String configRootDir, String filePattern,
        Class<C> confType, Function<C,List<T>> getList, Function<T,String> getId) {

        try(Stream<Path> walk = Files.walk(Paths.get(configRootDir))) {
            Pattern pattern = Pattern.compile(filePattern);

            return getLocalFilePaths(walk, pattern)
                .flatMap(p -> this.parseConfigFile(p, confType, getList))
                .collect(Collectors.toMap(getId, Function.identity()));

        } catch(IOException|UncheckedIOException ex) {
            logger.error("Specified config root directory not found.", ex);
            return Collections.emptyMap();
        }
    }

    /**
     * Read a given configuration file from local disk and map to specified config type
     *
     * @param configFile Path to config file on local disk
     * @param configType Configuration type (MyTypeConf.class or MyOtherTypeConf.class)
     * @param getList Configuration type specific list accessor method
     */
    private <T,C> Stream<T> parseConfigFile(
        Path configFile, Class<C> configType, Function<C,List<T>> getList) {

        try {
            C conf=Core.getMapper().readValue(Files.readString(configFile), configType);
            List<T> tRefs = getList.apply(conf);
            return tRefs.stream();
        } catch(IOException ex) {
            throw new UncheckedIOException(ex);
        }
    }

    /**
     * Get file paths of all matching files exist in configured streaming directory
     * and sub folders from disk.
     *
     * @param walk    Stream of paths in config root directory.
     * @param pattern Pattern to math when discovering files.
     * @return Stream of Path objects for all files matching the pattern.
     */
    private Stream<Path> getLocalFilePaths(Stream<Path> walk, Pattern pattern) {
        return walk.filter(Files::isRegularFile).filter(p -> {
            String fileName = p.getFileName().toString();
            Matcher matcher = pattern.matcher(fileName);
            return matcher.matches();
        });
    }
}