CODEWITHSUNDEEP
javamultithreadinghashmapsynchronization
Ionuț U
Ionuț U

Reputation: 45

Synchronized hashmap access

On my computer, using java 8, the following program won't stop even if map access is synchronized. Aren't those synchronized blocks enougth?

import java.util.HashMap;
import java.util.concurrent.TimeUnit;

// Broken! - How long would you expect this program to run?
public class StopThread {
    private static HashMap<String, String> stopRequested = new HashMap<String, String>();

    public static void main(String[] args) throws InterruptedException {
        Thread backgroundThread = new Thread(new Runnable() {
            public void run() {
                int i = 0;
                synchronized (stopRequested) {
                    while (stopRequested.get("stop") == null)
                        i++;
                }
                System.out.println(i);
            }
        });
        backgroundThread.start();
        TimeUnit.SECONDS.sleep(1);
        synchronized (stopRequested) {
            stopRequested.put("stop", "true");
        }
    }
}

Upvotes: 0

Views: 298

Answers (4)

Amer Qarabsa
Amer Qarabsa

Reputation: 6574

Yes that is expected, your backgroundThread is holding the lock before your main thread and it wont release it until the main thread writes "stop" to the map, the main thread needs the lock to write it the "stop" so basically this is a dead lock.

There are several ways to solve this deadlock, my guess is what you are trying to do is to see how many times you count before the main thread writes "stop" entry in your map. You can simply acquire and release your lock on each iteration of the loop which makes sense for your scenario.

while (stopRequested.get("stop") == null)
    synchronized (stopRequested) {
                i++;
     }

Another solution could be using concurrentHashMap, check this link for more details

Upvotes: 1

VGR
VGR

Reputation: 44308

You have two threads synchronizing on stopRequested. Only one synchronized block is permitted to run at any given time. Since it will almost always be the case that backgroundThread’s synchronized block runs first, it will never exit and thus will never allow any other thread to synchronize on stopRequested.

The wait and notify methods exist precisely to solve this problem:

    try {
        int i = 0;
        synchronized (stopRequested) {
            while (stopRequested.get("stop") == null) {
                stopRequested.wait();
                i++;
            }
        }
        System.out.println(i);
    } catch (InterruptedException e) {
        throw new RuntimeException(i);
    }

// ...

synchronized (stopRequested) {
    stopRequested.put("stop", "true");
    stopRequested.notify();
}

The reason this works is that wait() will temporarily, and atomically, release the synchronized lock, allowing another thread to synchronize on that object.

Note that wait() must be called in a loop which checks the condition being waited for, since wait() can occasionally return even if no other thread called notify(). This “spurious wakeup” is due to the nature of threads on some systems.

A well-behaved thread will place the entire wait-loop in a try/catch block, so the thread will exit when interrupted. An interrupt is a request from some other thread asking your thread to stop what it’s doing and exit cleanly.

Upvotes: 1

Ionuț U
Ionuț U

Reputation: 45

Thanks for all the answers. Indeed, this is a deadlock. A working synchronization is 

import java.util.HashMap;
import java.util.concurrent.TimeUnit;

// Broken! - How long would you expect this program to run?
public class StopThread {
    private static HashMap<String, String> stopRequested = new HashMap<String, String>();

    public static void main(String[] args) throws InterruptedException {
        Thread backgroundThread = new Thread(new Runnable() {
            public void run() {
                int i = 0;
                while (stopRequested())
                    i++;
                System.out.println(i);
            }
        });
        backgroundThread.start();
        TimeUnit.SECONDS.sleep(1);
        synchronized (stopRequested) {
            stopRequested.put("stop", "true");
        }
    }

    static boolean stopRequested()
    {
        synchronized (stopRequested) {
            return stopRequested.get("stop") == null;
        }
    }
}

Upvotes: 0

Jim Stansl
Jim Stansl

Reputation: 21

This will run forever. The while loop in the synchronized block is effectively infinite since it is entered first - thus prohibiting the second synchronized block from ever being entered.

Upvotes: 2

Related Questions