Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Critical problem with DefaultCDI and concurrency #285

Open
roneigebert opened this issue May 20, 2022 · 0 comments
Open

Critical problem with DefaultCDI and concurrency #285

roneigebert opened this issue May 20, 2022 · 0 comments
Labels

Comments

@roneigebert
Copy link
Member

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:
image

Thread-safe solution

DependencyMap.java

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

DefaultCDI.java

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

roneigebert added a commit to roneigebert/kikaha that referenced this issue May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant