Singleton created with enum type, problems with threads safety

Question

Good day, I have created Singleton :

import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedList;

public enum Singleton {
    FIRST_INSTANCE;

    String[] scrabbleLetters = {
            "a","a","a","a","a","a","a","a","a","b","b","b","b","b","b","b","b","b",
            "c","c","c","c","c","c","c","c","c","d","d","d","d","d","d","d","d","d","d",
    };

    private LinkedList<String> letterList = new LinkedList<>(Arrays.asList(scrabbleLetters));

    private Object lock = new Object();

    private Singleton() {
        Collections.shuffle(letterList);
    }

    public static Singleton getInstance() {
        return FIRST_INSTANCE;
    }

    public LinkedList<String> getLetterList() {
        synchronized (lock) {

        return FIRST_INSTANCE.letterList;
        }
    }

    public LinkedList<String> getTiles(int howManyTiles) {
        synchronized (lock) {

        LinkedList<String> tilesToSend = new LinkedList<>();
        for(int i=0; i<= howManyTiles; i++) {
            tilesToSend.add(FIRST_INSTANCE.letterList.remove(0));
        }
        return tilesToSend;

        }
    }

}

and I have tested it on threads safety with this example :

import java.util.LinkedList;

public class ScrabbleTest {
    public static void main(String[] args) {
        Runnable getTiles = () -> {

            System.out.println("In thread : " +Thread.currentThread().getName());
            Singleton newInstance = Singleton.getInstance();
            System.out.println("Instance ID: " + System.identityHashCode(newInstance));
            System.out.println(newInstance.getLetterList());

            LinkedList<String> playerOneTiles = newInstance.getTiles(7);
            System.out.println("Player : " + Thread.currentThread().getName() + playerOneTiles);
            System.out.println("Got Tiles for " + Thread.currentThread().getName());
        };

        new Thread(getTiles, "First").start();
        new Thread(getTiles, "Second").start();
    }
}

After executing it 10 times , I was sure that there is no problem, but when I run it last time I received this stack trace :

In thread : Second
In thread : First
Instance ID: 1380197535
Instance ID: 1380197535
[d, d, b, c, b, b, a, d, c, d, a, d, c, a, a, d, c, a, a, b, d, b, b, a, b, c, a, d, c, a, c, b, c, c, b, d, d]
Player : First[d, d, b, c, b, b, a, d]
Got Tiles for First
Exception in thread "Second" java.util.ConcurrentModificationException
    at java.util.LinkedList$ListItr.checkForComodification(Unknown Source)
    at java.util.LinkedList$ListItr.next(Unknown Source)
    at java.util.AbstractCollection.toString(Unknown Source)
    at java.lang.String.valueOf(Unknown Source)
    at java.io.PrintStream.println(Unknown Source)
    at ScrabbleTest.lambda$0(ScrabbleTest.java:10)
    at java.lang.Thread.run(Unknown Source)

This exception happens quit rarely, about 1 time for 20 executions. I have found that ConcurrentModificationException may be thrown by methods that have detected concurrent modification of an object when such modification is not permissible. In the code I have a lock that should prevent this situations, there is the same lock for changing and retrieving the list for synchronized blocks. I don't even imagine why this happens.

Answer 1

CME doesn't have as much to do with concurrency as the name might make you think. The most common occurrence for CME is in a single threaded context. In this case however, the threading is involved too.

Your problem comes from tilesToSend.add(FIRST_INSTANCE.letterList.remove(0)); in which you're modifying the letterList, but it's being iterated at the same time by println. Synchronization won't help here, since you'd have to synchronize a lot larger blocks than what is realistically possible.

Easy solution here is to return a copy of the list in getLetterList() like

return new LinkedList<>(FIRST_INSTANCE.letterList);

This way the original list can be modified by remove() while println is iterating the copy.