java AtomicInteger decmentAndGet не работает должным образом

Question

Кажется, что AtomicInteger DecmentAndGet не работает правильно в многопоточном режиме. Хотя incrementAndGet работает правильно. Я запускаю эту программу от Исполнителей и в 10 потоках. Каждый поток уменьшает значение в 5 раз в l oop, поэтому 10 потоков уменьшат его значение до -50. Но я не получаю значение -50. То же самое, если я делаю для вызова incrementAndGet тогда, он увеличивается до 50.

public class RunCountAtomic {

public static  AtomicInteger aValue = new AtomicInteger(0);

public void runLoop() {
    for(int i=0;i<5;i++) {
//      aValue.getAndIncrement();
            doSomeWork();
            aValue.decrementAndGet();
        }
    System.out.println(" Expected value is - (5 x 10 thread = -50) , actual is: " +aValue.get());
    }
    // Some dummy work!     
    public void doSomeWork() { try {Thread.sleep(100);  } catch (InterruptedException e) {}  } 
}

Результат показывает, что incrementAndGet работает правильно, но не decmentAndGet! У кого-нибудь из вас есть такое наблюдение? Или я мог что-то упустить в коде.

Ниже приведен тестовый код, который вызывает метод вышеприведенного класса:

[Обновлено согласно ответу на комментарий] Обновите мой код TestMainRL, чтобы использовать службу executor для его упрощения.

public static void main(String[] args) {
    int noOfThreads         = 10;
    ExecutorService e = Executors.newFixedThreadPool(noOfThreads); 

    List<Future<Boolean>>   fblist = new ArrayList<Future<Boolean>>(noOfThreads);  

    System.out.println("Starting");     
    // Run the async task in multiple threads.
    for (int i = 0; i<noOfThreads; i++) {
        TestMainRL t = new TestMainRL();
        fblist.add(t.asyncTask ( e));
    }

    //Just to wait all thread to complete.
    //try {for(Future<Boolean> fb: fblist) {fb.get();}} catch (Exception e1) {e1.printStackTrace();}
    e.shutdown();
    System.out.println("Completed!");

}

Кстати, ниже показан результат, показывающий, что последний поток не равен -50:

Starting
Completed!
Thread[pool-1-thread-1,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -42
Thread[pool-1-thread-2,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -42
Thread[pool-1-thread-6,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -46
Thread[pool-1-thread-7,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -50
Thread[pool-1-thread-5,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -49
Thread[pool-1-thread-3,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -49
Thread[pool-1-thread-4,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -49
Thread[pool-1-thread-10,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -46
Thread[pool-1-thread-8,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -46
Thread[pool-1-thread-9,5,main] Expected value is - (5 x 10 thread = -50) , actual is: -46

Получив ответ от @acm, я понял, что для достижения цели должен печатать только один поток -50 и это было бы возможно с кодом ниже, который не использует ReentrantLock с кодом ниже RunCountAtomi c:

public class RunCountAtomic {
    public static  AtomicInteger aValue = new AtomicInteger(0);
    public void runLoop() {
        int x = 0;
        for(int i=0;i<5;i++) {
           doSomeWork();
           x = aValue.decrementAndGet(); // decrement and store it in local x.
       }
       // Here at the time of this query post, I was printing aValue.get() instead of using stored x,
       // which will ensure that only one thread will have -50.
       System.out.println(Thread.currentThread()+" Value in thread " +x);
    }
    public void doSomeWork() { 
        try {
             Thread.sleep(100);
        } catch (InterruptedException e) {} 
    } // Some dummy work!
}

Поток, получивший -50, является последним потоком, который выполнил последний декремент. Но при выполнении system.out он может не печатать в конце.

Answer 1

Чего именно вы ожидаете? Все темы для возврата -50, последний поток для возврата -50? В коде, который вы предоставили, это не гарантируется.

AtomicInteger ведет себя как ожидалось. У вас есть гарантия, что хотя бы один из потоков вернет -50, и это правильно?

Ключевой момент: System.out.println -> PrintStream#println:

public void println(String x) {
    synchronized (this) {
        print(x);
        newLine();
    }
}

Таким образом, все ваши потоки вызывают это, но в этом типе блокировки нет справедливости. Таким образом, хотя последний поток, который достигнет этой блокировки, был со значением -50, он может быть не первым, который будет напечатан, поэтому у вас есть гарантия, что -50 будет напечатан, но в какой-то момент не последний.

Чтобы проверить объяснения, вы можете использовать ReentrantLock со справедливостью в истинном значении, что означает, что самый старый ожидающий поток будет пробужден первым. Измените RunCountAtomic следующим образом:

public class RunCountAtomic {
    private static final ReentrantLock lock = new ReentrantLock(true);
    public static AtomicInteger aValue = new AtomicInteger(0);

    public void runLoop() {
        for (int i = 0; i < 5; i++) {
            doSomeWork();
            aValue.decrementAndGet();
        }
        printlnValue(aValue.get());
    }

    private void printlnValue(int value) {
        lock.lock();
        try {
            System.out.println(" Expected value is - (5 x 10 thread = -50) , actual is: " + value);
        } finally {
            lock.unlock();
        }
    }

    public void doSomeWork() {
        try {
            Thread.sleep(100);
        } catch (InterruptedException e) {}
    }
}

Теперь вы гарантированно напечатаете -50 последним.