AtomValenceTool is not thread-safe: concurrent first use can observe a partially initialized valence table #1308

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;

import org.openscience.cdk.Atom;
import org.openscience.cdk.qsar.AtomValenceTool;

public final class AtomValenceRace {
    public static void main(String[] args) throws Exception {
        int n = 256;

        CountDownLatch ready = new CountDownLatch(n);
        CountDownLatch start = new CountDownLatch(1);
        AtomicReference<Throwable> failure = new AtomicReference<>();
        List<Thread> threads = new ArrayList<>();

        for (int i = 0; i < n; i++) {
            Thread thread = new Thread(() -> {
                ready.countDown();

                try {
                    start.await();

                    // "Co" is the last element inserted into the current table,
                    // which makes the partial-initialization window easy to see.
                    AtomValenceTool.getValence(new Atom("Co"));
                } catch (Throwable ex) {
                    failure.compareAndSet(null, ex);
                }
            });

            threads.add(thread);
            thread.start();
        }

        ready.await();
        start.countDown();

        for (Thread thread : threads) {
            thread.join();
        }

        if (failure.get() != null) {
            throw new AssertionError(failure.get());
        }
    }
}

if (valencesTable == null) {
    valencesTable = new HashMap<>();
    valencesTable.put(...);
    // remaining entries
}

private static final Map<String, Integer> VALENCES = createValences();

private static Map<String, Integer> createValences() {
    Map<String, Integer> values = new HashMap<>();
    values.put("H", 1);
    // remaining entries
    return Collections.unmodifiableMap(values);
}