When using static object lock doesn't work?

Question

I'm new at Concurrency, i'm trying to understand synchronized block:

public static int count1 = 0;
public static Object lock1 = new Object();

public static void add(){
        synchronized (lock1) {
            count1++;
        }
}

my problem is with lock1, it doesn't work, when the method starts printing the color, it prints them randomly, so i think the problem is in synchronized block, because i watch some tutorials about this, all of them say the lock object must be static so no interference happens, but here i don't see that, Why?

This is the method that prints the color of each thread:

public static void compute(){
        String color = null;
        switch (Thread.currentThread().getName()) {
            case "First Count Down":
                color = TextColors.ANSI_YELLOW;
                break;
            case "Second Count Down":
                color = TextColors.ANSI_MAGENTA;
                break;
        }
        for (int i=0;i<100;i++) {
            System.out.println(color + Thread.currentThread().getName() + "is Running");
            //add();
            add();
        }
    }

and this is the threads :

public static void main(String[] args) {

        Thread t1 = new Thread(new Runnable() {
            @Override
            public void run() {
                compute();
            }
        });
        t1.setName("First Count Down");

        Thread t2 = new Thread(new Runnable() {
            @Override
            public void run() {
                compute();
            }
        });
        t2.setName("Second Count Down");

        t1.start();
        t2.start();

        try{
            t1.join();
            t2.join();

        }catch (InterruptedException io){
            io.printStackTrace();

        }

        System.out.println("Count1 = " + count1 + " Count2 = " + count2);

    }

Sorry if my English is bad, i'm not a native speaker, thanks in advance

What do you mean by "lock not working" and "printing randomly"? What do you expect this program to do? — Joni, Mar 25 '20 at 00:19
The `synchronized` block only prevent multiple threads from executing the `count1++;` statement at the same time, but the lock is released at the end of the block, so there is nothing to prevent multiple threads from *alternating* the calls to `add()`. If you want a thread to have exclusive access for the duration of the `for` loop, then you need the `synchronized` block to *cover* the loop. — Andreas, Mar 25 '20 at 00:25
Also see this [answer](https://stackoverflow.com/a/9459743/2310289) — Scary Wombat, Mar 25 '20 at 00:37
If you don't want it 'random', why are you using threads? And what is `count2`? — user207421, Mar 25 '20 at 01:25
Having a more direct question would be helpful here. Explaining more about what you are expecting and what is the output here will also give more clarity when you say "printing randomly" and "lock not working" mean. — Parth Satra, Mar 25 '20 at 06:22

Janos Vinceller · Accepted Answer · 2020-03-25T01:15:52.217

First of all, I think now I understand your question. You're trying to print out your lines without interfering with the lines of the other threads.

To achieve this, you have to "protect" the part of your code, which prints out one line, not to get printed out another line at the same time from another thread.

The protection can be done by synchronizing those lines of code.

You are currently synchronizing only the addition to that counter.

Your lock (the object you're locking on, the one and only instance of that new Object()) is static and you do not change it in your code, so it must work.

public static Object lock1 = new Object();

You could make the variable to final to get immutable, but currently that's not the problem. I would recommend that though.

The lock means, that if any other thread is landing in (executing) the same line of code (the beginning of the synchronized block), they won't get execution until the blocking thread is giving up its lock. This is only true, if they holding and asking for the very same lock. And as you're only using the very same new Object() instance, your lock should be okay.

Currently your code is built up that the add() method basically waits until one of the threads is counting up one.

If you want to change it so that you're getting separately printed out lines, try to synchronize the "line printing block" like this:

    synchronized (lock1) {
        System.out.println(color + Thread.currentThread().getName() + "is Running");
        add();
    }

And let the counting not be synchronized.

private static void add(){
    count1++;
}

This would work but mostly you don't want to let your add() method unsynchronized. It could be executed from other threads, who knows. But for your case, those changes would help.

@HovercraftFullOfEels What exactly *is* the main question? – user207421 Mar 25 '20 at 01:26 — user207421, Mar 25 '20 at 01:26

When using static object lock doesn't work?

1 Answers1