4

How can I achieve that the while loop here is always executed exactly 100 times. When I execute the code, in rare cases it prints 99 or 98 lines on the console and not always 100, which I do not understand.

import java.util.ArrayList;
import java.util.concurrent.atomic.AtomicInteger;

public class Print implements Runnable {
    static AtomicInteger atomicInteger = new AtomicInteger(0);

    @Override
    public void run() {
        while (atomicInteger.getAndIncrement() < 100)
            System.out.println(Thread.currentThread());
    }

    public static void main(String[] args) throws InterruptedException {
        ArrayList<Thread> threads = new ArrayList<>();

        for (int i = 0; i < 5; i++)
            threads.add(new Thread(new Print()));

        for (Thread thread : threads)
            thread.start();

        for (Thread thread : threads)
            thread.join();
    }
}
Anne Maier
  • 299
  • 1
  • 8
  • 4
    This code should work because `getAndIncrement` is thread-safe, are you sure it prints only 98 or 99 times? – Karol Dowbecki Jun 08 '21 at 10:50
  • Thanks Karol. Could the IDE or other programs running in the background be affecting the output? I tried to reproduce the problem but was unable to. I had copied the console output in Excel and I am sure that it was once 98 and the other 99 lines. – Anne Maier Jun 08 '21 at 12:02
  • I couldn't reproduce your problem and the code looks correct, I don't know how you could have received 98 or 99 instead of 100. – Karol Dowbecki Jun 08 '21 at 12:03
  • 4
    The counting part looks ok to me. I have a tiny smidgen of doubt about the parallel println calls; the [description](https://docs.oracle.com/javase/7/docs/api/java/io/PrintStream.html#println(java.lang.Object)) suggests that printing the value and printing the line terminator are distinct actions, which I suppose *may* result in two values followed by two newlines - but then you'd have obviously-wrong output. – iggy Jun 08 '21 at 13:06
  • I agree with @iggy, this is likely an artefact with `println`. To confirm, try incrementing another `AtomicInteger` in the body of the `if` statement and observe its final value. – michid Oct 15 '22 at 10:57
  • [ExecutorCompletionService](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ExecutorCompletionService.html) ..since java 1.5!;) – xerx593 Oct 16 '22 at 08:44

2 Answers2

2

Unable to replicate your reported experience.

In my mind’s debugger, that seems correct as I cannot see any thread-safety fault in your code.

To automate further testing, I altered your code as follows. Rather than call System.out.println, I added the thread ID to a List of Long. I made the list a CopyOnWriteArrayList for thread-safety. I can programmatically detect if the resulting list is not exactly 100 elements in size.

package work.basil.demo;

import java.time.Instant;
import java.util.ArrayList;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;

public class Print implements Runnable
{
    static AtomicInteger atomicInteger = new AtomicInteger( 0 );
    static CopyOnWriteArrayList < Long > list = new CopyOnWriteArrayList <>();

    @Override
    public void run ( )
    {
        while ( atomicInteger.getAndIncrement() < 100 )
            //System.out.println(Thread.currentThread());
            list.add( Thread.currentThread().getId() );
    }

    public static void main ( String[] args ) throws InterruptedException
    {
        System.out.println( "INFO - demo starting. " + Instant.now() );
        for ( int cycle = 0 ; cycle < 1_000_000 ; cycle++ )
        {
            ArrayList < Thread > threads = new ArrayList <>();

            for ( int i = 0 ; i < 5 ; i++ )
                threads.add( new Thread( new Print() ) );

            for ( Thread thread : threads )
                thread.start();

            for ( Thread thread : threads )
                thread.join();

//            System.out.println( "list.size() = " + list.size() );
//            if ( list.size() == 100 ) { System.out.println( "DEBUG list.size() = " + ( list.size() ) ); }
            if ( list.size() != 100 ) { System.out.println( "DEBUG list.size() = " + ( list.size() ) ); }
        }
        System.out.println( "INFO - demo done. " + Instant.now() );
    }
}

When run on a Mac mini (2018) 3 GHz Intel Core i5 with six real cores and no hyper-threading, and 32 GB 2667 MHz DDR4, using Java 16 within IntelliJ. Running cycle to a million takes about 5 minutes.

INFO - demo starting. 2021-06-08T22:11:56.010181Z
INFO - demo done. 2021-06-08T22:16:26.982616Z

ExecutorService

By the way, in modern Java we rarely need to address the Thread class directly. Instead, use the Executors framework added to Java 5.

Here is revised version of the code above, rejiggered to use an executor service.

public static void main ( String[] args ) throws InterruptedException
{
    System.out.println( "INFO - demo starting. " + Instant.now() );
    for ( int cycle = 0 ; cycle < 1_000_000 ; cycle++ )
    {
        ExecutorService executorService = Executors.newFixedThreadPool( 5 );

        int countTasks = 5;
        for ( int i = 0 ; i < countTasks ; i++ )
        {
            executorService.submit( new Print2() );
        }

        executorService.shutdown();
        executorService.awaitTermination( Duration.ofMinutes( 7 ).toSeconds() , TimeUnit.SECONDS );

//            System.out.println( "list.size() = " + list.size() );
//            if ( list.size() == 100 ) { System.out.println( "DEBUG list.size() = " + ( list.size() ) ); }
        if ( list.size() != 100 ) { System.out.println( "DEBUG list.size() = " + ( list.size() ) ); }
    }
    System.out.println( "INFO - demo done. " + Instant.now() );
}
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
0
    static AtomicInteger atomicInteger = new AtomicInteger(0); change to
    AtomicInteger atomicInteger = new AtomicInteger(1);// 0->1 no static
  
    while (atomicInteger.getAndIncrement() < 100) //change to:
    while (atomicInteger.getAndIncrement() != 101)//100->101

remove "static" for the variable atomicInteger. if you need to get exactly 1...100 (not 0...99) set the atomic value to 1 and in the loop set !=101 my print https://gyazo.com/6762c78ae44c0b06d05e316768114167

The comment clarified the question: the key point is the use of statics. use a semaphore

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

public class Print implements Runnable {
    static AtomicInteger atomicInteger = new AtomicInteger(1);

    private final Semaphore semaphore;

    public Print(final Semaphore semaphore) {
        this.semaphore = semaphore;
    }

    @Override
    public void run() {
        int count = 0;
        try {
            semaphore.acquire();
            while ((count = atomicInteger.getAndIncrement()) != 101) {
            System.out.println(
            "COUNT::" + count + "::" + Thread.currentThread());
        }
           atomicInteger.set(1);
            
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        } finally {
            semaphore.release();
        }

    }

    public static void main(String[] args) throws InterruptedException {
        List<Thread> threads = new ArrayList<>();
        Semaphore semaphore = new Semaphore(1);
        for (int i = 0; i < 5; i++)
            threads.add(new Thread(new Print(semaphore)));
        for (Thread thread : threads)
            thread.start();
        for (Thread thread : threads)
            thread.join();
    }
}
mailtime
  • 111
  • 1
  • 8
  • That won't work correctly, if thread 1 arrives and increments to 101 (and receive 100), thread 2 can arrive and increment to 102 (and receive 101), and so on for threads 3 - 5. – Mark Rotteveel Oct 14 '22 at 09:14
  • Please remove the word "static for the variable atomicInteger. This is my print for 3threads and while !=5 gyazo.com/bb8a712e1d32934c362e1a662d0b3a80 – mailtime Oct 15 '22 at 10:20
  • The use static is required for the problem the OP tries to implement (or they would need to pass a single instance of the `AtomicInteger` to each instance of `Print`), as the shared use of the `AtomicInteger` is the whole point. – Mark Rotteveel Oct 15 '22 at 12:04
  • I supplemented the answer if static is important in the context of this question – mailtime Oct 15 '22 at 13:03