Необходимо понять проблему в использовании кода AtomicInteger в многопоточной среде - PullRequest
2 голосов
/ 26 февраля 2020

В одном из интервью мне был задан вопрос о кодировании, и мне пришлось найти проблему в этом коде и предложить правильное решение.

Пожалуйста, найдите ниже весь код:

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;

public class Atomic {

    static AtomicInteger count = new AtomicInteger(0);
    static int counter = 0;

    public static class Runnable extends Thread {

    public void run() {
        while (count.getAndSet(1) != 0) {
            try {
                Thread.sleep(3000);
            } catch (Exception e) {

            }
        }
        counter = counter + 1;
        count.set(0);
    }

}

public static void main(String[] args) {
    ExecutorService executor = Executors.newFixedThreadPool(10);
    for (int i = 0; i < 10; i++) {
        Runnable runnable = new Runnable();
        executor.execute(runnable);
    }
    executor.shutdown();
}

}

Этот код работает правильно. Но вопрос в том, есть ли какая-то проблема в этом коде, если число потоков увеличивается или я запускаю For l oop почти 10000 раз.

Я пытался найти проблему, но не смог ее найти .

1 Ответ

3 голосов
/ 26 февраля 2020

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


Во-первых, переменная counter не обновляется безопасно. Несколько потоков не имеют гарантированной видимости последнего записанного значения; у вас также нет гарантии, что ни один другой поток не обновил свое значение между чтением и записью.

Простое решение этого: замените counter на AtomicInteger и используйте getAndIncrement или incrementAndGet для увеличения.


Во-вторых, public static class Runnable extends Thread { является чрезвычайно сомнительным.

  1. Не скрывайте имена общеизвестных Java классов (это скрывается java.lang.Runnable)
  2. Не расширяйте Thread напрямую, особенно когда все вы нужно java.lang.Runnable для добавления выполнения с ExecutorService.

Более подходящим объявлением класса будет:

public static class MyRunnable implements Runnable {

(или как вы хотите его назвать)

Или вы можете просто объявить анонимный класс:

executor.execute(new Runnable() { /* body */ });

Или вы можете просто объявить лямбду:

executor.execute(() -> { /* body */ });

В-третьих, count не на самом деле, кажется, здесь нет очевидной цели. Лог c запускаемого объекта выглядит следующим образом:

  • Если «flag» имеет значение false:
    1. Установить «flag» на true
    2. Увеличить переменную
    3. Установите «флаг» в false
  • В противном случае:
    1. Подождите 3 секунды
    2. Попробуйте еще раз

count играет здесь роль «флага». Фактически это просто AtomicBoolean.

Но вам вообще не нужна отдельная переменная count, если вы сделаете counter an AtomicInteger:

while (true) {
  int current = counter.get();
  if (counter.compareAndSet(current, current + 1)) {
    // Nothing else is trying to update "current" at the same time:
    // we updated it. Stop.
    break;
  }

  // Something else is trying to update at the same time.
  // Sleep for 3 seconds.
  Thread.sleep(3000);
}
...