0

I am learning synchronization in Java. I know this is a very basic question, but I can't find out why I can't get 24000 as a result after running the code to "count".

public class Example extends Thread {

    private static int count;

    public static void main(String[] args) throws InterruptedException {
        Example t1 = new Example();
        Example t2 = new Example();
        
        t1.start();
        t2.start();
        
        t1.join();
        t2.join();
        
        System.out.println(count);
    }

    public void run() {
        for (int i = 0; i < 12000; i++) {
            addToCount();
        }
    }

    public synchronized void addToCount() {
        Example.count++;
    }
}
Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
gerg
  • 19
  • 1
  • 6
  • 1
    count is static, so it is a class variable, and not an instance variable – jr593 Feb 10 '21 at 11:44
  • 3
    You don't share an instance, so the `synchronized` keyword has no effect as both `t1` and `t2` have their own lock, instead of using one lock. Either share one instance of `Example` or make `addToCount` method `static` (the faster way to get what you want). Then read more about multi-threading. – Kayaman Feb 10 '21 at 12:09
  • Your a question is not clear. What exactly do you expect as a result? And what exactly is your actual result? – Basil Bourque Feb 10 '21 at 17:29

2 Answers2

2

synchronized on separate objects does not coordinate between them

When I run your code, I get results such as 21788, 24000, 20521.

The reason for the various results is that your synchronized is a lock on each of two separate objects. The objects referenced by t1 and t2 each have their own lock (monitor). Within each thread's execution, executing the synchronized method addToCount grabs the monitor for that particular thread, that particular Example object. So effectively the synchronized has no effect. Your use of the synchronized keyword is not coordinating between the two objects, only within each single object.

For more info, see comment by Mark Rotteveel below, and see the Oracle Tutorial on Synchronized Methods. And read the Brian Goetz book linked below.

++ is not atomic

So in your code, no purpose is served by making addToCount synchronized.

Each of your two Example objects is a separate thread, each accessing a shared resource, the static int count variable. Each is grabbing the current value, and sometimes they are simultaneously grabbing and incrementing the very same value. For example, they both may the value 42, each adding one gets a result of 43, and each put 43 into that variable.

The ++ operator in Java is not atomic. Within the source code, we programmers think of it as a single operation. But it is actually multiple operations. See Why is i++ not atomic?.

Conceptually (not literally), you can think of your Example.count++; code as being:

int x = Example.count ;  // Make a copy of the current value of `count`, and put that copy into `x` variable.
x = ( x + 1 ) ;          // Change the value of `x` to new incremented value.
Example.count = x ;      // Replace the value of `count` with value of `x`.

While the multiple steps of fetch-increment-replace are being executed, at any point the operations may be suspended as the execution of that thread pauses for some other thread to execute a while. The thread might have completed the first step, fetching a copy of 42, then paused when thread suspends. During this suspension, the other thread may grab the same 42 value, increment it to 43, and replace back into count. When the first thread resumes, having already grabbed 42, this first thread also increments to 43 and stores back into count. The first thread will have no idea that the second thread slipped in there to already increment and store 43. So 43 ends up being stored twice, using two of our for loops.

This coinidence, where each thread steps on the toes of the other, is unpredictable. On each run of this code, the scheduling of the threads may vary based on current momentary conditions within the host OS and the JVM. If our result is 21788, then we know that run experienced 2,212 collisions ( 24,000 - 21,788 = 2,212 ). When our result is 24,000, we know we happened to have no such collisions, by sheer arbitrary luck.

You have yet another problem as well. (Concurrency is tricky.) Read on.

Visibility problem

Because of CPU architecture, the two threads might see different values for the same single static variable. You will need to study visibility in the Java Memory Model.

AtomicInteger

You can solve both the visibility and synchronization problems by using the Atomic… classes. In this case, AtomicInteger. This class wraps an integer value, providing a thread-safe container.

Mark the AtomicInteger field final to guarantee that we have one and only one AtomicInteger object ever, preventing re-assignment.

final private static AtomicInteger count = new AtomicInteger() ;

To perform addition, call a method such as incrementAndGet. No need to mark your own method as synchronized. The AtomicInteger handles that for you.

public void  addToCount() {
    int newValue = Example.count.incrementAndGet() ; 
    System.out.println( "newValue " + newValue + " in thread " + Thread.currentThread().getId() + "." ) ;
}

With this kind of code, two threads incrementing the same AtomicInteger object for 12,000 times each results in a value of 24,000.

For more info, see this similar Question, Why is incrementing a number in 10 Java threads not resulting in a value of 10?.

Executor service

One more issue with your code is that in modern Java, we generally no longer address the Thread class directly. Instead, use the executors framework added to Java 5.

Part of what makes your code tricky is that it mixes the thread-management (being a subclass of Thread) with being a worker-bee trying to get a job done (incrementing a counter). This violates the single-responsibility principle that generally leads to better designs. By using an executor service, we can separate the two responsibilities, thread-management versus counter-incrementing.

Project Loom

Using an executor service has been shown on many many pages of Stack Overflow already. So search to learn more. Instead, I will show the simpler future approach if and when Project Loom technology becomes a part of Java. Experimental releases based on early-access Java 17 are available now.

try-with-resources syntax waits for submitted tasks

In Loom, the ExecutorService is AutoCloseable. This means we can use try-with-resources syntax. The try block exits only after all submitted tasks are done/failed/canceled. And when exiting the try block, the executor service is automatically closed for us.

Here is our Incremental class that holds the static AtomicInteger named count. The class includes a method to increment that atomic object. And this class is a Runnable with a run method to do your 12,000 loops.

package work.basil.example;

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

public class Incremental implements Runnable
{
    // Member fields
    static final public AtomicInteger count = new AtomicInteger();  // Make `public` for demonstration purposes (not in real work).

    public int addToCount ( )
    {
        return this.count.incrementAndGet();  // Returns the new incremented value stored as payload within our `AtomicInteger` wrapper.
    }

    @Override
    public void run ( )
    {
        for ( int i = 1 ; i <= 12_000 ; i++ )
        {
            int newValue = this.addToCount();
            System.out.println( "Thread " + Thread.currentThread().getId() + " incremented `count` to: " + newValue + " at " + Instant.now() );
        }
    }
}

And code from a main method, to utilize that class. We instantiate a ExecutorService via the factory methods found in Executors. Then within a try-with-resources we submit two instances of Incremental to each be run in their own thread.

As per your original Question, we still have two objects, two threads, twelve thousands increment commands in each thread, and results stored in a single static variable named count.

// Exercise the `Incremental` class by running two instances, each in its own thread.
System.out.println( "INFO - `main` starting the demo. " + Instant.now() );
Incremental incremental = new Incremental();
try (
        ExecutorService executorService = Executors.newVirtualThreadExecutor() ;
)
{
    executorService.submit( new Incremental() );
    executorService.submit( new Incremental() );
}

System.out.println( "INFO - At this point all submitted tasks are done/failed/canceled, and executor service is shutting down. " + Instant.now() );
System.out.println( "DEBUG - Incremental.count.get()  = " + Incremental.count.get() );  // Access the static `AtomicInteger` object.
System.out.println( "INFO - `main` ending. " + Instant.now() );

When run, your output might look like this:

INFO - `main` starting the demo. 2021-02-10T22:38:06.235503Z
Thread 14 incremented `count` to: 2 at 2021-02-10T22:38:06.258267Z
Thread 14 incremented `count` to: 3 at 2021-02-10T22:38:06.274143Z
Thread 14 incremented `count` to: 4 at 2021-02-10T22:38:06.274349Z
Thread 14 incremented `count` to: 5 at 2021-02-10T22:38:06.274551Z
Thread 14 incremented `count` to: 6 at 2021-02-10T22:38:06.274714Z
Thread 16 incremented `count` to: 1 at 2021-02-10T22:38:06.258267Z
Thread 16 incremented `count` to: 8 at 2021-02-10T22:38:06.274916Z
Thread 16 incremented `count` to: 9 at 2021-02-10T22:38:06.274992Z
Thread 16 incremented `count` to: 10 at 2021-02-10T22:38:06.275061Z
…
Thread 14 incremented `count` to: 23998 at 2021-02-10T22:38:06.667193Z
Thread 14 incremented `count` to: 23999 at 2021-02-10T22:38:06.667197Z
Thread 14 incremented `count` to: 24000 at 2021-02-10T22:38:06.667204Z
INFO - At this point all submitted tasks are done/failed/canceled, and executor service is shutting down. 2021-02-10T22:38:06.667489Z
DEBUG - Incremental.count.get()  = 24000
INFO - `main` ending. 2021-02-10T22:38:06.669359Z

Read the excellent and classic book Java Concurrency in Practice by Brian Goetz, et al.

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
  • Although this answer provides an alternative solution, it does not explain what is wrong with the code of the question (static variable, while the synchronized method is non-static, so both threads don't use the same monitor). – Mark Rotteveel Feb 10 '21 at 19:19
  • @MarkRotteveel Fair point, thanks for the criticism. I added two more sections to expand on your comment. – Basil Bourque Feb 10 '21 at 22:05
2

As others have hinted at, the reason is to do with what you are actually synchronising on when you declare a method as synchronized:

  • If the method is static, then you are synchronizing on the class; only one thread at a time will be able to enter the method.
  • If the method is not static, then you are synchronizing on the individual instance of your class. Multiple threads can call the same method concurrently on different instances of your class (but not on the same instance). Since each thread has its own instance in your case, they can call the method concurrently, each on its separate instance.

So the solution is basically to be consistent: either (a) have one shared static variable (as you have), and then make the method static; or (b), let each individual instance have its own variable, and then sum the variables at the end of the operation to get the overall count. (As a variant of (b), you could also have multiple threads referring to and accessing the same instance.)

Neil Coffey
  • 21,615
  • 7
  • 62
  • 83