Search code examples
javathread-safetyatomic-long

How to ensure the correctness of AtomicLong addAndGet result


I want calculate current percentage in my muli-thread download programme.But there is a strange problem . The lastDownloadSize during the second download must be the sum of write and lastDownloadSize of lastDown. example

There is my code

private long getDownloadSize() {
    synchronized (this) {
        final AtomicLong totalWriteCount = new AtomicLong(0);
        final AtomicLong lastDownloadSize = new AtomicLong(0);
        for (DownloadTask task : downloadTasks) {
            final long writeCount = task.getWriteCount();
            totalWriteCount.addAndGet(writeCount);
            final long downloadSize = task.getPosition().getDownloadSize();
            lastDownloadSize.addAndGet(downloadSize);
        }
        System.out.println("=====  writeCount : " + totalWriteCount + "lastDownloadSize : " + lastDownloadSize);
        return totalWriteCount.addAndGet(lastDownloadSize.get());
    }
}

Solution

  • Your totalWriteCount and lastDownloadSize variables are local variables of the getDownloadSize() method. In that case, it does not make sense to use AtomicLong, because only a single thread can access them.

    What you probably meant, is to make totalWriteCount and lastDownloadSize members of your class:

    class SomeClass {
        // ...
        final AtomicLong totalWriteCount = new AtomicLong(0);
        final AtomicLong lastDownloadSize = new AtomicLong(0);
        // ...
    
        private long getDownloadSize() {
            synchronized (this) {
                for (DownloadTask task : downloadTasks) {
                    final long writeCount = task.getWriteCount();
                    totalWriteCount.addAndGet(writeCount);
                    final long downloadSize = task.getPosition().getDownloadSize();
                    lastDownloadSize.addAndGet(downloadSize);
                }
                System.out.println("=====  writeCount : " + totalWriteCount + "lastDownloadSize : " + lastDownloadSize);
                return totalWriteCount.addAndGet(lastDownloadSize.get());
            }
        }
    }
    

    However, also in that case, if they are only accessed from within synchronized(this) blocks, you do not need to use AtomicLongs, because the synchronized block already makes sure that they are only accessed by a single thread at the same time.