How do I make this thread safe?

Question

is this thread Safe? I keep getting different results every time I'm not sure how to make this thread safe. My goal is to compute the amount of primes starting from 5 -10000 but doing this using a thread for each computation. I would like to keep track of the total amount of prime numbers. if I run the code below I get 21 sometimes and 22 sometimes.

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

public class Main {
     private static AtomicInteger count =new AtomicInteger(0);
    private static ExecutorService ex = Executors.newFixedThreadPool(100);
   
     public static void main(String []args) throws IOException {
        int x =5;
        while(x < 90){
            final int index =x;
           ex.execute(new Runnable(){
                public void run(){
                    synchronized(count){
                        if(isPrime(index)){
                            count.incrementAndGet();
                        }
                    }
                }
            });
            x++;
            //new Thread(task).start();
        }
        ex.shutdown();
        System.out.println(count.get()+ "this how many numbers are  prime");
     }
     
    public static boolean isPrime(int n)
    {
        // Corner cases
        if (n <= 1)
            return false;
        if (n <= 3)
            return true;
 
        // This is checked so that we can skip
        // middle five numbers in below loop
        if (n % 2 == 0 || n % 3 == 0)
            return false;
 
        for (int i = 5; i * i <= n; i = i + 6)
            if (n % i == 0 || n % (i + 2) == 0)
                return false;
 
        return true;
    }
}

"*is this thread Safe? I keep getting different results every time I'm not sure how to make this thread safe.*" - If the program in and of itself should behave deterministic, and you get different results when executing it in parallel, then it is most likely not thread-safe. --- Hint: `count++` is not an atomic operation. --- Hint 2: Take a look at [`AtomicInteger`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/concurrent/atomic/AtomicInteger.html) — Turing85, Mar 18 '21 at 21:46
@Turing85 i tried AtomicInteger same issue i updated the post with latest changes Does the isPrime function need to be Synchronized? — Dylan Shapiro, Mar 18 '21 at 21:50
I never said that this would be the only problems. Please take a look at [the documentation of `ExecutorServce::shutdown`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/concurrent/ExecutorService.html#shutdown()). — Turing85, Mar 18 '21 at 21:57

score 0 · Answer 1 · answered Mar 18 '21 at 22:28

adding the function shutdownAndAwaitTermination(ExecutorService ex) fixed the issue rather then just calling ex.shutdown(); i called shutdownAndAwaitTermination(ex);

 public static void shutdownAndAwaitTermination(ExecutorService ex) {
    ex.shutdown(); // Disable new tasks from being submitted
    try {
     // Wait a while for existing tasks to terminate
     if (!ex.awaitTermination(60, TimeUnit.SECONDS)) {
       ex.shutdownNow(); // Cancel currently executing tasks
       // Wait a while for tasks to respond to being cancelled
       if (!ex.awaitTermination(60, TimeUnit.SECONDS))
           System.err.println("Pool did not terminate");
     }
   } catch (InterruptedException ie) {
     // (Re-)Cancel if current thread also interrupted
     ex.shutdownNow();
     // Preserve interrupt status
     Thread.currentThread().interrupt();
   }
 }

You should remove the first `awaitTermination(...)` call. It will just be sleeping for `60` seconds because you haven't shutdown the `ExecutorService`. If you really want to wait for the background tasks for 120 seconds then just change the `awaitTermination` after the `shutdownNow()` to be 120. — Gray, Mar 20 '21 at 05:11

How do I make this thread safe?

1 Answers1