In Java Critical Sections, What Should I Synchronize On

In Java critical sections, what should I synchronize on?

First, note that the following code snippets are identical.

public void foo() {
    synchronized (this) {
        // do something thread-safe
    }
}

and:

public synchronized void foo() {
    // do something thread-safe
}

do exactly the same thing. No preference for either one of them except for code readability and style.

When you do synchronize methods or blocks of code, it's important to know why you are doing such a thing, and what object exactly you are locking, and for what purpose.

Also note that there are situations in which you will want to client-side synchronize blocks of code in which the monitor you are asking for (i.e. the synchronized object) is not necessarily this, like in this example :

Vector v = getSomeGlobalVector();
synchronized (v) {
    // some thread-safe operation on the vector
}

I suggest you get more knowledge about concurrent programming, it will serve you a great deal once you know exactly what's happening behind the scenes. You should check out Concurrent Programming in Java, a great book on the subject. If you want a quick dive-in to the subject, check out Java Concurrency @ Sun

Why does this critical section not work in Java?

I ran this code myself and when you have the synchronized blocks set up, it works (the yield() call returns quickly), but when you remove them, it doesn't (the app hangs, the yield() call seemingly never returning).

This is expected behaviour, more or less. If this code did return swiftly from that yield() call, that would ALSO be expected behaviour.

Welcome to the Java Memory Model.

Every thread gets an unfair coin. it is unfair, in that it is free to screw with you, and flip the same way every time, making it look like something works reliably, juuuust for it to fail when you're giving that demo to that important client. The basic rule is simple. If a thread ever flips the coin and the way your code runs differs depending on how it lands, you lost the game: You have a bug, and writing a test that catches it is virtually impossible. Writing threading code like this is therefore an act that requires extreme discipline.

The thread will flip the coin anytime it reads or writes a field. Each thread has a custom local clone copy of every field of every object* which it will read from and write to. But everytime it does so, it flips that evil coin. If the coin lands heads, it will arbitrarily copy its values to some or all other thread's copies, or copy the values from some other thread's local copy to itself. On tails, it doesn't do that and just uses its local copy.

In this case, you're observing a (near) endless coinflip of tails. In real life, you don't get a few million tails in a row, but did I mention the coin is unfair? It's 'normal' here. But the crucial point is: The VM makes no guarantees about that coin flip at all - a VM that flips heads every time (thus having yield() return quickly) is just as good, will pass the TCK, etcetera.

Thus: If ever the coin flip matters to how your code runs, you've messed up.

And you've done so here: worker-thread (the thread, not the object) reads its copy (which is false and remains false), and main-thread sets its copy of workerthread's (the object, not the thread)'s done flag to true. 1 field, but 2 cached copies, and now we're waiting for a coinflip of heads before that worker-thread ever manages to see what main-thread did.

Fortunately, we can force the VM to not flip that coin, and the route to do so is to establish a comes-before relationship.

The java memory model has written down a series of rules which establish that the VM will then acknowledge that 'event A happened before event B', and will guarantee that anything A did will be visible to B. No coin flip - B's copy will necessarily reflect what A did.

There's a long list but the important ones are:

• imperative: Within a single thread, any statement that runs before another came-before that other. This is the 'duh, obviously' one: In { foo(); bar(); }, anything foo does is visible to bar, because same thread.

• synchronized: Whenever a thread exits a synchronize-on-object-X block, that 'happens-before' any thread entering a synchronize-on-object-X block, if the thread entering does so afterwards.

• volatile: volatile is a keyword you can put on a field. This is a bit trickier; volatile is defined in terms of CB/CA too, but it's a little easier to consider that any writing to volatile variables forces the coin to flip heads, writing out the update to every thread's copy. That makes volatile seem amazing, but it comes at quite a cost: volatile is rather slow, and not quite as useful as you might initially think, because you can't use it to set up atomic operations.

• Thread start: Whenever you start a thread, anything the thread that starts the new thread did before calling .start() is visible to the new thread immediately.

Of course, if some method internally synchronizes on things, by combining the synchronized rule and the imperative rule, then that method effectively also serves as a comes-before-establishing factor. You should use this - a bunch of JDK methods are defined to do so!

So, toss the synchronized blocks back in, thus establishing CB/CA between main-thread writing the done flag and worker-thread reading it, and the code works (and will in fact work on every JVM, regardless of OS, CPU, phase of the moon, or VM vendor). Without it, this code can do whatever it wants. Exit, or not, or exit after 4 hours, or exit only when your winamp switches songs. All's fair then, because you wrote code that depends on the result of a coin flip, so it's on you. Alternatively, mark the field 'volatile', that'll do it too.

---

So, how do you write multicore java code if it's this much of a minefield??

The true answer is: You mostly don't. It's too complicated. Instead, you do this:

• isolate: If no thread interacts with any other at all (nothing reads/writes fields except stuff entirely local to that thread's goingson), then trivially all the coin flips in the world aren't going to change anything, so do that. Set up the thread properly before you start it, let the thread run to its end, and have it communicate the thing it produced (if you need a return value at all) via a safe channel, and don't worry about it.
• streamline comms: Instead of communicating via fields (here your threads 'communicate'; main communicates to worker in the form of that boolean), communicate via a channel that is designed for concurrent control: Use a DB (thread A writes to DB, thread B also does so and the same tables and rows as A; DBs have facilities to manage this), or a message queue. This can be a full blown library like rabbitMQ, or a collection type from java.util.concurrent, such as BlockingQueue. These have all solved the CB/CA issues for you.
• rely on a framework: A web framework has many threads and will call your 'web handler' code on one appropriately. The handlers chat via DB, the web framework takes care of all the threading: All your CPU cores are zooming along and you never need to worry about any coinflips. You can also use the framework at the opposite end and use e.g. fork/join or the concept of stream->map->filter->collect (a.k.a. MapReduce).

*) It doesn't really, I'm giving you a mental model of how to think about it so that you won't write this bug. The point is, it MAY make a clone.

java synchronizing access to different critical sections

What you need is an exclusive lock on criticalMethod and shareable locks on methods method1 and method2. The simplest way to achieve this is to use java.util.concurrent.locks.ReentrantReadWriteLock as below:

public class CriticalSections {
    private final ReadWriteLock lock = new ReentrantReadWriteLock();

    public void mainMethod(boolean b) {
        if (b) {
            method1();
        } else {
            criticalMethod();
            method2();
        }
    }

    private void criticalMethod() {
        Lock writeLock = lock.writeLock();
        writeLock.lock();
        try {
            System.out.println("in criticalMethod");
        } finally {
            writeLock.unlock();
        }
    }

    private void method1() {
        Lock readLock = lock.readLock();
        readLock.lock();
        try {
            System.out.println("in method1");
        } finally {
            readLock.unlock();
        }
    }

    private void method2() {
        Lock readLock = lock.readLock();
        readLock.lock();
        try {
            System.out.println("in method2");
        } finally {
            readLock.unlock();
        }
    }
}

If you're worried about performance, you can also take a look at java.util.concurrent.locks.StampedLock (Java 8+).

synchronization : Threads execute two critical sections in same order

As I understand Critical Section #2 MUST be executed in the same order as Critical Section #1

If thread T1 executes block1 before thread T2, then T1 should execute block2 before T2. There are more than two threads.

Then a Queue might be used to ensure the order of execution.

private Object lock = new Object();
private Queue<Thread> threadQueue = new ArrayDeque<>();

// https://stackoverflow.com/questions/32353283/synchronization-threads-execute-two-critical-sections-in-same-order
public void executeCriticalSectionsInOrder() throws InterruptedException {
    // Critical Section #1
    synchronized (lock){
        // synchronized code #1

        // Add self to queue
        threadQueue.add(Thread.currentThread());
    }

    // {lot of code where synchronization not necessary}

    // Critical Section #2
    synchronized (lock) {
        //All the threads that executed block1 before this thread should have already executed this block.
        // Wait turn
        Thread t = threadQueue.element(); // Do not remove until it is self
        while (t != Thread.currentThread()) {
            lock.wait();
            // After sleep try again
            t = threadQueue.element();
        }
        // Verified own turn. Update status
        threadQueue.remove();

        // synchronized code #2

        lock.notifyAll(); // Awake any waiting thread after exiting section.
    }

However If one thread dies/exits without removing itself from the queue, then following threads will be blocked indefinetely. Maybe add a finally block to do the housekeeping?

Note: In Nicholas Robinson's answer a position order was suggested instead of a queue, which seems slightly more efficient.

Java - Synchronized block works differently than method

Per the JLS 8.4.3.6. synchronized Methods:

A synchronized method acquires a monitor (§17.1) before it executes.

For a class (static) method, the monitor associated with the Class object for the method's class is used.

For an instance method, the monitor associated with this (the object for which the method was invoked) is used.

In the synchronized (getClass()) block you synchronize on the Class object, thus all instances of ThreadSynchronous are serialized.

When you make the instance method synchronized, you are synchronizing only on that instance (the this reference).

Avoid synchronized(this) in Java?

I'll cover each point separately.

1. Some evil code may steal your lock (very popular this one, also has an
   "accidentally" variant)

   I'm more worried about accidentally. What it amounts to is that this use of this is part of your class' exposed interface, and should be documented. Sometimes the ability of other code to use your lock is desired. This is true of things like Collections.synchronizedMap (see the javadoc).

2. All synchronized methods within the same class use the exact same
   lock, which reduces throughput

   This is overly simplistic thinking; just getting rid of synchronized(this) won't solve the problem. Proper synchronization for throughput will take more thought.

3. You are (unnecessarily) exposing too much information

   This is a variant of #1. Use of synchronized(this) is part of your interface. If you don't want/need this exposed, don't do it.

---

Related Topics