Synchronization in Java: Threads Created but Not Executed Synchronously

[Kieu Thoai]

I have a Java code snippet that involves creating multiple threads to execute a method, but I'm having trouble understanding the synchronization aspect of it. Here's the code:

public class SheepManager {
    private int sheepCount = 0;
    private void incrementAndReport() {
        System.out.print((++sheepCount)+" ");
    }
    public static void main(String[] args) {
        ExecutorService service = null;
        try {
            service = Executors.newFixedThreadPool(20);
            SheepManager manager = new SheepManager();
            synchronized (manager){
                for(int i=0; i<10; i++)
                    service.submit(() -> manager.incrementAndReport());
            }
        } finally {
            if(service != null) service.shutdown();
        }
    }
}

In this code, I've synchronized the creation of threads by placing the service.submit() calls within a synchronized block. However, I'm not sure if this guarantees that the threads will execute in a synchronized manner. Is this approach correct for ensuring synchronized execution, or is there a better way to achieve this?

Any guidance on how to properly synchronize the execution of the threads created in this code would be greatly appreciated.

[Stephan van Hulst]

Welcome to CodeRanch!

The use of the synchronized keyword in your example is pointless. The piece of code that is being synchronized is only ever executed by a single thread, and no other thread synchronizes on the manager object.

Synchronization only has an effect when two different threads access pieces of code protected by the same object.

[Mike Simmons]

Stephan van Hulst wrote:Synchronization only has an effect when two different threads access pieces of code protected by the same object.

Elaborating on this.. there is a place in your code where multiple threads access data from the same object.  And that place should be synchronized, or otherwise protected somehow, to be thread-safe.  But the place where you submit the tasks is not the place, because the tasks don't actually execute there.  The place you need to protect is the code that actually executes.  This can be done several ways:

                    service.submit(() -> synchronized(manager) { manager.incrementAndReport() });

Or

    private void incrementAndReport() {
        synchronized (this) {
            System.out.print((++sheepCount)+" ");
        }
    }

Or (perhaps the best):

    private synchronized void incrementAndReport() {
        System.out.print((++sheepCount)+" ");
    }

These are all equivalent in that they synchronize on the same object, which is the one that hold the mutable data that needs to be protected - namely, the manager.sheepCount field.

[Roland Mueller]

Solution # one only works when the right part of the lambda is included into an addition pair of curly brackets. Otherwise I received the following error:

$ javac SheepManager.java && ( java SheepManager ; echo ) SheepManager.java:16: error: illegal start of expression                        synchronized (manager) { manager.incrementAndReport(); }                        ^ SheepManager.java:17: error: illegal start of expression                );                ^ 2 errors

import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;

public class SheepManager {
    private int sheepCount = 0;
    private void incrementAndReport() {
        System.out.print((++sheepCount)+" ");
    }
    public static void main(String[] args) {
        ExecutorService service = null;
        try {
            service = Executors.newFixedThreadPool(20);
            SheepManager manager = new SheepManager();
                for(int i=0; i<10; i++)
                    service.submit(() -> 
                        synchronized (manager) { manager.incrementAndReport(); } 
                );
        } finally {
            if(service != null) service.shutdown();
        }
    }
}

                    service.submit(() -> /* start of lambda */ { 
                        synchronized (manager) { manager.incrementAndReport(); }  } /* end of lambda */
                );

[Paul Clapham]

Or you could perhaps use an AtomicInteger, which internalizes the synchronization of changes to its internal integer value. You'd use whichever of the "getAndIncrement" or "incrementAndGet" methods corresponds to your usage of the ++ operator.

[Mike Simmons]

Yes, definitely - AtomicInteger is an excellent fit, here.  There are other ways, too, including using a java.util.concurrent.locks.Lock - but AtomicInteger is simple to use and does just what is needed.

[Kieu Thoai]

Stephan van Hulst wrote:Welcome to CodeRanch!

The use of the synchronized keyword in your example is pointless. The piece of code that is being synchronized is only ever executed by a single thread, and no other thread synchronizes on the manager object.

Synchronization only has an effect when two different threads access pieces of code protected by the same object.

In this book they said "  We’ve
synchronized the creation of the threads but not the execution of the threads. In this
example, each thread would be created one at a time, but they may all still execute and
perform their work at the same time"  I don't understand the difference between synchronized create thread and execution thread, I think synchronzied thread ensures there is only 1 thread accessing

[Stephan van Hulst]

The synchronized keyword protects a block of code by acquiring a lock. Two different threads can not execute a synchronized block of code if those two synchronized blocks use the same lock.

In your original code, the piece of code being protected is:

synchronized (manager){
  for(int i=0; i<10; i++)
    service.submit(...);
}

The lock that is used for this block of code is the object referenced by the manager variable. If two threads were to execute the same synchronized block, or a different synchronized block that also locks on manager, then one of the threads will have to wait until the other has released the lock on the manager object.

See how I removed the lambda expression inside the submit() call? That's to make it clear that the body of the lambda expression is NOT protected by the synchronized block. Why? Because the thread that executes the body of the lambda expression never executes the synchronized keyword. Think about it, we only have two kinds of threads in this specific example:

- The "main" thread: Executes the main() method, and so it also executes the synchronized keyword. All the code it executes until it leaves the synchronized block is protected. The call to incrementAndReport() is not protected, because it is not executed by the main thread.

- The "increment and report" threads. These execute the call to incrementAndReport(). They do not acquire any locks, because they never execute the synchronized keyword.

So there is only ONE thread that executes the synchronized keyword. That means that the synchronized keyword is completely unnecessary for this example, because it only actually does anything if TWO OR MORE threads execute it, using the same lock.

[Kieu Thoai]

Thank you for answer , it very helpfull
One more question when i used service = Executors.newFixedThreadPool(20); -> has it will create 20 threads at the same time
But when i used syncrhonized as above it creates them one by one right ?

[Stephan van Hulst]

Kieu Thoai wrote:Thank you for answer , it very helpfull
One more question when i used service = Executors.newFixedThreadPool(20); -> has it will create 20 threads at the same time

If I'm honest, I don't know whether newFixedThreadPool(20) immediately creates all threads, or whether it waits with creating a thread until a task comes available. Either way, none of those threads can start until you've submitted a task using service.submit().

But when i used syncrhonized as above it creates them one by one right ?

The main thread submits each task one by one. But it has nothing to do with the synchronized keyword. Like I said before, the use of the synchronized keyword in your specific example code has absolutely zero effect.

There is only a single thread executing the call to service.submit(). A single thread always does things one at a time, regardless of whether it's in a synchronized block or not.