ConcurrentModificationException

[t1]

I run my integration tests in parallel and every test needs a different configuration. In order to make this possible, I wrote a Thread Local Config Source.

But now my test often fail with a ConcurrentModificationException when I iterate over the Config#getPropertyNames().

At first, I thought that I must not reuse the Config instance in parallel threads, but in the Spec it says: "All methods in the ConfigProvider, ConfigProviderResolver and Config implementations are thread safe and reentrant.".

I think this could be a bug? Any help would be appreciated.

[dmlloyd]

I'd have to know more about your particular code, but CME isn't actually just about threads, it's about any case where you have an "open" iterator when a modification is made, even if that happens in the same thread. So, it's hard to say what exactly is going wrong without looking at your actual code, but one possibility is that you're modifying the configuration while you are iterating over it (i.e. inside of your iteration loop).

[t1]

I've tried to create a reproducer, but that's tough, as it fails only sporadically. And I can't share the production code.

But the ConcurrentModificationException happens in Smallrye GraphQL in the GraphQLClientsConfiguration class, so I can share that part:

/**
 * The wrapper that stores configuration of all GraphQL clients.
 */
public class GraphQLClientsConfiguration {

    private static final Map<ClassLoader, GraphQLClientsConfiguration> INSTANCES = new WeakHashMap<>();
    private static volatile boolean singleApplication = false;
    Config mpConfig = ConfigProvider.getConfig();

    /**
     * This needs to be set to true if the runtime only supports a single deployment.
     */
    @SuppressWarnings("unused")
    public static void setSingleApplication(boolean singleApplication) {
        GraphQLClientsConfiguration.singleApplication = singleApplication;
    }

    public static GraphQLClientsConfiguration getInstance() {
        ClassLoader key = singleApplication ? null : Thread.currentThread().getContextClassLoader();
        return INSTANCES.computeIfAbsent(key, x -> new GraphQLClientsConfiguration());
    }

    /**
     * The map storing the configs of each individual client.
     * <p>
     * The key in this map is:
     * For typesafe clients, the client's `configKey` or, if the `configKey` is not defined, the fully qualified class name
     * For dynamic clients, always the client's `configKey`.
     */
    private final Map<String, GraphQLClientConfiguration> clients = new HashMap<>();

    public GraphQLClientsConfiguration() {
        // Store configuration found in config properties
        Set<String> detectedClientNames = new HashSet<>();
        for (String propertyName : mpConfig.getPropertyNames()) {   // <------------ it fails here!
            // assume that the name of a configured client can consist of
            // uppercase and lowercase letters, numbers, dashes and underscores
            if (propertyName.matches("^[A-Za-z0-9-_.$]+/mp-graphql/.+$")) {
                String key = propertyName.substring(0, propertyName.indexOf("/mp-graphql"));
                if (!clients.containsKey(key)) {
                    detectedClientNames.add(key);
                }
            }
        }
        for (String clientName : detectedClientNames) {
            clients.put(clientName, readConfigurationByKey(clientName));
        }
    }

    private GraphQLClientConfiguration readConfigurationByKey(String clientName) {
        ...
    }

    ...
}

This happens in the constructor. The stacktrace starts like this:

java.util.ConcurrentModificationException
	at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1597)
	at java.base/java.util.HashMap$KeyIterator.next(HashMap.java:1620)
	at java.base/java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1054)
	at io.smallrye.graphql.client.impl.GraphQLClientsConfiguration.<init>(GraphQLClientsConfiguration.java:54)
	at io.smallrye.graphql.client.impl.GraphQLClientsConfiguration.lambda$getInstance$0(GraphQLClientsConfiguration.java:34)
	at java.base/java.util.Map.computeIfAbsent(Map.java:1054)
	at io.smallrye.graphql.client.impl.GraphQLClientsConfiguration.getInstance(GraphQLClientsConfiguration.java:34)
	at io.smallrye.graphql.client.vertx.typesafe.VertxTypesafeGraphQLClientBuilder.applyConfigFor(VertxTypesafeGraphQLClientBuilder.java:195)
	at io.smallrye.graphql.client.vertx.typesafe.VertxTypesafeGraphQLClientBuilder.build(VertxTypesafeGraphQLClientBuilder.java:155)
        ...

The javadoc of the getPropertyNames() method states explicitly: "The returned instance is thread safe and may be iterated concurrently. The individual iterators are not thread-safe." So the code looks perfectly fine to me.

@dmlloyd: Maybe you can spot something? I'm running out of options... other than running the tests only sequentially, i.e. living with a terribly slow CI/CD pipeline.

[radcortez]

I've tried to reproduce it with no luck either, but I think there is a case when something like this can happen, so I'll keep trying.

Thinking a bit about what you are trying to achieve, there are other issues. For instance, we cache the list of property names (because it is expensive to calculate), meaning that if the tests rely on a single instance of SmallRyeConfig and dynamically change a ConfigSource the list of property names may not reflect that change. Getting the value programmatically is not cached, so that works.