Critical problem with DefaultCDI and concurrency

[roneigebert]

When use DefaultCDI "load" to retrieve an instance dynamically in some rare ocasions we have an lock.. This only ocurrs on and concurrent threads environment because of the the DefaultCDI and DependencyMap classes..

DependencyMap.java uses an HashSet (lockedDependencies attribute) and DefaultCDI.java is using and HashMap on createDefaultProvidedData method.. Both are not thread safe..

Test case:

package kikaha.core.cdi.helpers;

import kikaha.core.cdi.CDI;
import lombok.SneakyThrows;
import lombok.val;
import org.junit.Test;

import java.nio.channels.IllegalBlockingModeException;
import java.nio.channels.IllegalChannelGroupException;
import java.nio.channels.IllegalSelectorException;
import java.util.*;
import java.util.concurrent.Executors;

import static org.junit.Assert.*;

public class DependencyMapTest {

    static boolean shouldLoad = false;

    List<Object> objects = Arrays.asList( Integer.MAX_VALUE, Long.MAX_VALUE, Double.MAX_VALUE, Boolean.FALSE, Float.MAX_VALUE,
            new RuntimeException(), new IllegalAccessError(), new IllegalArgumentException(), new IllegalMonitorStateException(),
            new IllegalStateException(), new IllegalThreadStateException(), new IllegalAccessException(), new IllformedLocaleException(),
            new IllegalFormatPrecisionException(1), new IllegalFormatWidthException(1), new IllegalFormatCodePointException(1),
            new IllegalChannelGroupException(), new IllegalSelectorException(), new IllegalBlockingModeException());

    @Test
    @SneakyThrows
    public void multiThreadsLockSimulation(){
        val dependencyMap = new DependencyMap( createDefaultProvidedData() );
        val pool = Executors.newCachedThreadPool();
        for ( val object : objects ) {
            val clazz = object.getClass();
            val instanceCollection = Collections.singletonList(object);
            pool.submit(() -> {
                try {
                    System.out.println("Starting thread for class " + clazz);
                    while (true) {
                        if (shouldLoad)
                            break;
                        Thread.sleep(1);
                    }
                    dependencyMap.put(clazz, instanceCollection);
                    dependencyMap.unlock(clazz);
                    System.out.println("Ending thread for class " + clazz);
                } catch (Throwable e) {
                    System.out.println("ERR " + e);
                }
            });
        }
        Thread.sleep( 2000 );
        shouldLoad = true;
        Thread.sleep( 2000 );
        System.out.println( "Check..." );
        for ( val object : objects ) {
            val clazz = object.getClass();
            try {
                val loadedCollection = dependencyMap.get(clazz);
                assertNotNull("No lock but null for class " + clazz, loadedCollection);
                assertEquals(object, first(loadedCollection));
            } catch ( DependencyMap.TemporarilyLockedException lockError ){
                fail( "Lock exception for class " + clazz );
            }
        }
    }

    /** Same contents from DefaultCDI#createDefaultProvidedData */
    protected Map<Class<?>, Iterable<?>> createDefaultProvidedData() {
        // XXX: 50% of the solution
        // final Map<Class<?>, Iterable<?>> injectable = new ConcurrentHashMap<>();
        final Map<Class<?>, Iterable<?>> injectable = new HashMap<>();
        injectable.put( CDI.class, new SingleObjectIterable<>( this ) );
        return injectable;
    }

    private <T> T first(Iterable<T> instances) {
        return instances.iterator().next();
    }

}

Running this code, most of the time we have different erros.. Some examples: java.lang.AssertionError: Lock exception for class class java.lang.Integer java.lang.AssertionError: Lock exception for class class java.lang.Long java.lang.AssertionError: No lock but null for class class java.lang.Double java.lang.AssertionError: No lock but null for class class java.lang.IllegalAccessException

This problem is critical because when this happends there is a infinite loop when use load:

Thread-safe solution

DependencyMap.java

Alter from new HashSet<>() to ConcurrentHashMap.newKeySet()

DefaultCDI.java

Alter from new HashMap<>() to new ConcurrentHashMap<>()