JAVA synchronized block not working in a multithreaded environment

Question

I have the below code and trying to understand why the synchronization is not achieved here:

class Main {
    Thread t1 = new OpenServerSockets();
    t1.start();
}

public class OpenServerSockets extends Thread {
    @Override
    public void run() {
        while(true){
            Thread t = new ClientResponder(clientSocket, dis, dos);
            t.start();
        }
    }

public class ClientResponder extends Thread {
    @Override
    public void run(){
        synchronized(this) {
            // some other static method call in another class.
        }
    }
}

The synchronized block gets called by multiple threads at the same time. Why is it so? Isn't synchronized block used this way not supposed to ensure mutual exclusion execution of code?

you probably have more than one instance of ClientResponder, so this will vary — Andrew
– Andrew, Commented Oct 2, 2019 at 7:41
Ok, I got the point but what is a static object and how do we use one? — Pankaj
– Pankaj, Commented Oct 2, 2019 at 7:49
@Pankaj check posted the answer. An alternate approach is to pass lock object while creating the thread or pass ReentrantLock which provides the advanced features. — Govinda Sakhare
– Govinda Sakhare, Commented Oct 2, 2019 at 7:51
Okay, so what I understood is that when we use synchronized(ClassName.class) , here ClassName.class is treated as a static object? — Pankaj
– Pankaj, Commented Oct 2, 2019 at 7:57

Govinda Sakhare · Accepted Answer · 2019-10-02 08:14:13Z

2

I can see the lock is on this.

Thread t1 = new ClientResponder();
Thread t2 = new ClientResponder();

Now for t1 and t2 this is referring to a different objects. You need a class level lock.

public class ClientResponder extends Thread {
    @Override
    public void run(){
        synchronized(ClientResponder.class) {
            // some other static method call in another class.
        }
    }
}

The better approach would be to use ReentrantLock. It provides advanced features which syncronized doesn't provide

class ClientResponder extends Thread {

   public static final ReentrantLock lock = new ReentrantLock ();

    @Override
    public void run() {
        lock.lock ();
        // task.
        lock.unlock ();
    }
}

A quick read for the reference: Java Synchronized Block for .class

Rentrant Lock: https://javarevisited.blogspot.com/2013/03/reentrantlock-example-in-java-synchronized-difference-vs-lock.html

edited Oct 2, 2019 at 8:14

answered Oct 2, 2019 at 7:58

Govinda Sakhare

5,8506 gold badges41 silver badges86 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Pankaj Over a year ago

Thanks @Govinda for the answer. Will be helpful if you can share some more insights for the Reentrant lock or a link to a good read. Thanks!

T.J. Crowder · Accepted Answer · 2019-10-02 07:56:01Z

0

As has been covered in the comments, you're synchronizing on different objects, so there's no coordination between the threads.

Instead, you need to have the threads share an object that they synchronize on. You could create that object in Main or OpenServerSockets and then pass it into the constructor of ClientResponder, and then ClientResponder could synchronize on that object.

But there are other possible issues:

It's not clear why OpenServerSockets should be a Thread
It seems odd that OpenServerSockets constantly spawns new threads in a tight loop
You're wrapping your entire ClientResponder run method in a single synchronized block. That means that for the thread's entire useful life it's holding the lock. That seems unlikely to be what you need.
In general, it's best practice to implement Runnable rather than to extend Thread, and then pass an instance of the class implementing Runnable into new Thread.

Re #2: ClientResponder's run should instead probably look something like this:

public void run(){
    while (this.running) {
        synchronized (this.objectToSynchronizeOn) {
            // Do one unit of work that requires synchronization
        }
    }
}

That way, each of the threads gets a chance to do something in-between the work done by other threads.

answered Oct 2, 2019 at 7:56

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

3 Comments

Pankaj Over a year ago

Thanks for the answer Crowder. I have posted an edited snippet of the code just to ask where I had doubts.

T.J. Crowder Over a year ago

@Pankaj - Sure, of course, and that's good. A key thing when doing a minimal reproducible example, though, is to make sure it is representative of your actual code. If your actual code doesn't have the entire run method in the synchronized block, for instance, the MRE shouldn't either (even if it just has comments or something).

Pankaj Over a year ago

Thanks @Crowder, dint heard of the MRE concept before. It's a cool thing.

Collectives™ on Stack Overflow

JAVA synchronized block not working in a multithreaded environment

2 Answers 2

1 Comment

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related