JAVA синхронизированный блок не работает в многопоточной среде

Question

У меня есть приведенный ниже код, и я пытаюсь понять, почему здесь не достигается синхронизация:

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.
        }
    }
}

Синхронизированный блок вызывается несколькими потоками одновременно. Почему это так? Разве синхронизированный блок, используемый таким образом, не предназначен для обеспечения взаимного исключения выполнения кода?

Answer 1

Я вижу, что блокировка включена this.

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

Теперь для t1 и t2 this относится к другим объектам. Вам нужна блокировка на уровне класса.

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

Лучшим подходом будет использование ReentrantLock. Он предоставляет расширенные функции, которые syncronized не предоставляет

class ClientResponder extends Thread {

   public static final ReentrantLock lock = new ReentrantLock ();

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

Краткое чтение для справки: Синхронизированный блок Java для .class

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

Answer 2

Как уже говорилось в комментариях, вы синхронизируете разные объекты, поэтому нет никакой координации между потоками.

Вместо этого вам нужно, чтобы потоки совместно использовали объект, с которым они синхронизируются. Вы можете создать этот объект в Main или OpenServerSockets, а затем передать его в конструктор ClientResponder, а затем ClientResponder может синхронизироваться с этим объектом.

Но есть и другие возможные проблемы:

1. Непонятно, почему OpenServerSockets должно быть Thread
2. Кажется странным, что OpenServerSockets постоянно порождает новые темы в узкой петле
3. Вы заключаете весь метод ClientResponder run в один блок synchronized. Это означает, что на протяжении всей полезной жизни потока он удерживает блокировку. Кажется, вряд ли это то, что вам нужно.
4. В целом, лучше реализовать Runnable, чем расширять Thread, а затем передать экземпляр класса, реализующего Runnable, в new Thread.

Re # 2: ClientResponder run должно вместо этого выглядеть примерно так:

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

Таким образом, каждый из потоков получает шанссделать что-то промежуточное между работой, выполняемой другими потоками.