Синглтон создан с типом enum, проблемы с безопасностью потоков

Добрый день, я создал 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;

        }
    }

}

и я проверил его на безопасность потоков с помощью этого примера:

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();
    }
}

Выполнив его 10 раз, я был уверен, что проблемы нет, но когда я запускал его в последний раз, я получил эту трассировку стека:

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)

Это исключение бывает редко, примерно 1 раз на 20 исполнений. Я обнаружил, что ConcurrentModificationException может вызываться методами, которые обнаруживают одновременное изменение объекта, когда такое изменение недопустимо. В коде у меня есть блокировка, которая должна предотвращать такие ситуации, есть такая же блокировка для изменения и получения списка для синхронизированных блоков. Я даже не представляю, почему это происходит.


person Maksim    schedule 22.09.2017    source источник
comment
Ваш код не является потокобезопасным. Период. Проблема возникает с System.out.println(newInstance.getLetterList());, который не защищен никаким синхронизированным блоком.   -  person Mark Rotteveel    schedule 22.09.2017
comment
Метод getLetterList() имеет синхронизированный блок, разве этого недостаточно?   -  person Maksim    schedule 22.09.2017
comment
Нет, проблема возникает после возврата списка (и выхода из синхронизированного блока), вам либо нужно скопировать список, либо вам нужно преобразовать список в строку в синхронизированном блоке и напечатайте это вместо этого. Однако ваш подход чреват множеством других проблем, которые вы должны решить вместо этого. Спросите себя, почему вы вообще используете здесь синглтон. Что делать, если вы хотите сделать несколько игр в одном прогоне и т. д. и т. д. Это решение не является правильным, и это проблема, которую вы должны решить вместо этого.   -  person Mark Rotteveel    schedule 22.09.2017


Ответы (1)


CME не имеет столько общего с параллелизмом, как может показаться из названия. Чаще всего CME встречается в однопоточном контексте. Однако в этом случае также задействована резьба.

Ваша проблема исходит из tilesToSend.add(FIRST_INSTANCE.letterList.remove(0));, в котором вы изменяете letterList, но в то же время повторяется println. Синхронизация здесь не поможет, так как вам придется синхронизировать гораздо большие блоки, чем это реально возможно.

Простое решение здесь - вернуть копию списка в getLetterList(), например

return new LinkedList<>(FIRST_INSTANCE.letterList);

Таким образом, remove() может изменить исходный список, пока println повторяет копию.

person Kayaman    schedule 22.09.2017