Присвоение объекта полю, определенному вне синхронизированного блока - это потокобезопасно?

Question

Что-нибудь не так с безопасностью потока этого Java-кода? Потоки 1-10 добавляют числа с помощью sample.add (), а потоки 11-20 вызывают removeAndDouble () и выводят результаты в стандартный вывод. В глубине души я вспоминаю, что кто-то сказал, что назначение элемента таким же образом, как я это сделал в методе removeAndDouble () с использованием его вне синхронизированного блока, может быть поточно-ориентированным. Чтобы компилятор мог оптимизировать инструкции, чтобы они выполнялись не по порядку. Это тот случай здесь? Является ли мой метод removeAndDouble () небезопасным?

Что-то еще не так с точки зрения параллелизма с этим кодом? Я пытаюсь лучше понять параллелизм и модель памяти с Java (1.6 и выше).

import java.util.*;
import java.util.concurrent.*;

public class Sample {

    private final List<Integer> list = new ArrayList<Integer>();

    public void add(Integer o) {
        synchronized (list) {
            list.add(o);
            list.notify();
        }
    }

    public void waitUntilEmpty() {
        synchronized (list) {
            while (!list.isEmpty()) {
                try { 
                    list.wait(10000);  
                 } catch (InterruptedException ex) { }
            }
        }
    }

    public void waitUntilNotEmpty() {
        synchronized (list) {
            while (list.isEmpty()) {
                try { 
                    list.wait(10000);  
                 } catch (InterruptedException ex) { }
            }
        }
    }

    public Integer removeAndDouble() {
        // item declared outside synchronized block
        Integer item; 
        synchronized (list) { 
            waitUntilNotEmpty();
            item = list.remove(0);
        }
        // Would this ever be anything but that from list.remove(0)?
        return Integer.valueOf(item.intValue() * 2);
    }

    public static void main(String[] args) {
        final Sample sample = new Sample();

        for (int i = 0; i < 10; i++) {
            Thread t = new Thread() {
                public void run() {
                    while (true) {
                        System.out.println(getName()+" Found: " + sample.removeAndDouble());
                    }
                }
            };
            t.setName("Consumer-"+i);
            t.setDaemon(true);
            t.start();
        }

        final ExecutorService producers = Executors.newFixedThreadPool(10);
        for (int i = 0; i < 10; i++) {
            final int j = i * 10000;
            Thread t = new Thread() {
                public void run() {
                    for (int c = 0; c < 1000; c++) {
                        sample.add(j + c);
                    }
                }
            };
            t.setName("Producer-"+i);
            t.setDaemon(false);
            producers.execute(t);
        }

        producers.shutdown();
        try {
            producers.awaitTermination(600, TimeUnit.SECONDS);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

        sample.waitUntilEmpty();        
        System.out.println("Done.");
    }
}

Answer 1

Мне кажется, что это безопасно.Вот мои рассуждения.

Каждый раз, когда вы получаете доступ к list, вы делаете это синхронизировано.Это замечательно.Даже если вы извлекаете часть list в item, то к item не обращаются из нескольких потоков.

Пока вы получаете доступ только к list во время синхронизации, вы должны хорошо(в вашем текущем дизайне.)

Answer 2

Ваша синхронизация в порядке, и не приведет к каким-либо неупорядоченным проблемам выполнения.

Однако я замечаю некоторые проблемы.

Во-первых, ваш waitUntilEmpty метод будет гораздо более своевременным , если вы добавите list.notifyAll() после list.remove(0) в removeAndDouble. Это устранит до 10 секунд задержки в вашем wait(10000).

Во-вторых, ваш list.notify в add(Integer) должен быть notifyAll, потому что notify пробуждает только один поток, и он может пробудить поток, ожидающий внутри waitUntilEmpty вместо waitUntilNotEmpty.

В-третьих, ничто из вышеперечисленного не является предельным для жизнеспособности вашего приложения, поскольку вы использовали ограниченные ожидания, но если вы сделаете два вышеуказанных изменения, ваше приложение будет иметь лучшую многопоточную производительность (waitUntilEmpty), и ограниченные ожидания станут ненужными может стать простым старым безошибочным ожиданием.

Answer 3

Ваш код как есть на самом деле является потокобезопасным. Обоснование этого состоит из двух частей.

Первое - это взаимное исключение. Ваша синхронизация правильно гарантирует, что только один поток за раз будет изменять коллекции.

Второй связан с вашей заботой о переупорядочении компилятора. Вы обеспокоены тем, что компиляция может фактически изменить порядок присваивания, в котором она не будет поточно-ориентированной. Вам не нужно беспокоиться об этом в этом случае. Синхронизация в списке создает отношение «происходит раньше». Все удаления из списка происходит - перед записью в Integer item. Это говорит компилятору, что он не может переупорядочить запись в элемент в этом методе.

Answer 4

Ваш код является поточно-ориентированным, но не параллельным (как параллельно). Поскольку доступ ко всему осуществляется с помощью единой блокировки взаимного исключения, вы сериализуете весь доступ, в результате доступ к структуре является однопоточным.

Если вам требуется функциональность, описанная в вашем рабочем коде, пакет java.util.concurrent уже предоставляет реализации BlockingQueue с массивом (фиксированного размера) и (растущими) связанными списками. Их очень интересно изучать, по крайней мере, для идей реализации.