Почему этот класс не является потокобезопасным?

94
class ThreadSafeClass extends Thread
{
     private static int count = 0;

     public synchronized static void increment()
     {
         count++;
     }

     public synchronized void decrement()
     {
         count--;
     }
}

Может ли кто-нибудь объяснить, почему вышеуказанный класс не является потокобезопасным?

das kinder
источник
6
Я не знаю о Java, но похоже, что каждый из этих методов по отдельности потокобезопасен, но у вас может быть поток в каждом из методов одновременно. Возможно, если у вас есть единственный метод, который принимает bool ( increment), он будет потокобезопасным. Или если вы использовали какой-то блокирующий объект. Как я уже сказал, я не знаю о Java - мой комментарий основан на знании C #.
Вай Ха Ли
См. Stackoverflow.com/questions/7805192/…
Тагир Валеев
Я тоже не очень хорошо знаю Java, но для синхронизации доступа к статической переменной synchronizedследует использовать только в статических методах. Поэтому, по моему мнению, даже если вы удалите incrementметод, он все равно не является потокобезопасным, поскольку два экземпляра (которые имеют только синхронизированный доступ через один и тот же экземпляр) могут вызывать метод одновременно.
Онур
4
Это потокобезопасно, если вы никогда не создаете экземпляр класса.
Бенджамин Грюнбаум
1
Как вы думаете, почему это не потокобезопасно.
Raedwald

Ответы:

134

Поскольку это incrementметод, staticон будет синхронизироваться с объектом класса для ThreadSafeClass. decrementМетод не является статическим и синхронизировать экземпляр используется для его вызова. То есть они будут синхронизироваться на разных объектах, и, таким образом, два разных потока могут выполнять методы одновременно. Так как ++и --операции не являются атомарными класс не поточно.

Кроме того, поскольку countis static, изменение его метода decrementсинхронизированного экземпляра небезопасно, поскольку его можно вызывать в разных экземплярах и countодновременно изменять таким образом.

К. Эрландссон
источник
12
Вы можете добавить, так как countIS static, имеющий метод экземпляра decrement()является неправильным, даже если не было никакого static increment()способа, так как два потока может ссылаться decrement()на разных случаях модифицирующих тот же счетчик одновременно.
Хольгер
1
Возможно, это хорошая причина предпочесть использовать synchronizedблоки в целом (даже для всего содержимого метода) вместо использования модификатора метода, т.е. synchronized(this) { ... }и synchronized(ThreadSafeClass.class) { ... }.
Бруно
++и --не являются атомарными даже наvolatile int . Synchronizedзаботится о проблеме чтения / обновления / записи с помощью ++/ --, но staticключевое слово здесь является ключевым . Хороший ответ!
Крис Сайрефис
Re, изменение [статического поля] из ... синхронизированного метода экземпляра неверно : нет ничего неправильного по сути в доступе к статической переменной изнутри метода экземпляра, и нет ничего плохого по своей сути в доступе к ней из synchronizedметода экземпляра. Только не ожидайте, что "synchronized" в методе экземпляра обеспечит защиту статических данных. Единственная проблема здесь в том, что вы сказали в своем первом абзаце: методы используют разные блокировки в попытке защитить одни и те же данные, и это, конечно, не обеспечивает никакой защиты.
Соломон Слоу,
23

У вас есть два синхронизированных метода, но один из них статический, а другой - нет. При доступе к синхронизированному методу, в зависимости от его типа (статический или нестатический), другой объект будет заблокирован. Для статического метода блокировка будет помещена на объект Class, в то время как для нестатического блока блокировка будет помещена в экземпляр класса, который запускает метод. Поскольку у вас есть два разных заблокированных объекта, у вас может быть два потока, которые одновременно изменяют один и тот же объект.

Slimu
источник
14

Может ли кто-нибудь объяснить, почему вышеуказанный класс не является потокобезопасным?

  • increment будучи статическим, синхронизация будет выполняться в самом классе.
  • decrementбудучи не статическим, синхронизация будет выполняться при создании экземпляра объекта, но это ничего не защищает, поскольку countстатично.

Я хотел бы добавить, что для объявления поточно-безопасного счетчика я считаю, что самый простой способ - использовать AtomicIntegerвместо примитивного int.

Позвольте мне перенаправить вас к информации о java.util.concurrent.atomicпакете.

Жан-Франсуа Савар
источник
7

Другие ответы довольно хорошо объясняют причину. Я просто резюмирую synchronized:

public class A {
    public synchronized void fun1() {}

    public synchronized void fun2() {}

    public void fun3() {}

    public static synchronized void fun4() {}

    public static void fun5() {}
}

A a1 = new A();

synchronizedвключен fun1и fun2синхронизируется на уровне экземпляра объекта. synchronizedon fun4синхронизируется на уровне объекта класса. Что значит:

  1. Когда 2 потока вызывают a1.fun1()одновременно, последний вызов будет заблокирован.
  2. При одновременном вызове потока 1 a1.fun1()и потока 2 a1.fun2()последний вызов будет заблокирован.
  3. Когда поток 1 вызывает a1.fun1()и поток 2 вызывает a1.fun3()одновременно, без блокировки, 2 метода будут выполняться одновременно.
  4. Когда нить 1 звонок A.fun4(), если другие потоки называют A.fun4()или A.fun5()в то же время, последние вызовы будут заблокированы , так как synchronizedна одном fun4уровне класса.
  5. Когда поток 1 вызывает вызов A.fun4()потока 2 a1.fun1()одновременно, без блокировки, 2 метода будут выполняться одновременно.
кодерц
источник
6
  1. decrementблокирует другой объект, incrementчтобы они не мешали друг другу работать.
  2. Вызов decrementодного экземпляра блокирует другое, чем вызов decrementдругого экземпляра, но они влияют на одно и то же.

Первый означает, что перекрывающиеся вызовы incrementи decrementмогут привести к отмене (правильной), увеличению или уменьшению.

Второе означает, что два перекрывающихся вызова в decrementразных экземплярах могут привести к двойному (правильному) или единственному уменьшению.

Джон Ханна
источник
4

Поскольку два разных метода, один - это уровень экземпляра, а другой - уровень класса, поэтому вам нужно заблокировать 2 разных объекта, чтобы сделать его ThreadSafe

jaleel_quest
источник
1

Как объясняется в других ответах, ваш код не является потокобезопасным, поскольку статический метод increment()блокирует монитор классов, а нестатический метод decrement()блокирует монитор объектов.

Для этого примера кода существует лучшее решение без synchronzedиспользования ключевых слов. Вы должны использовать AtomicInteger для обеспечения безопасности потоков.

Потокобезопасное использование AtomicInteger:

import java.util.concurrent.atomic.AtomicInteger;

class ThreadSafeClass extends Thread {

    private static AtomicInteger count = new AtomicInteger(0);

    public static void increment() {
        count.incrementAndGet();
    }

    public static void decrement() {
        count.decrementAndGet();
    }

    public static int value() {
        return count.get();
    }

}
Равиндра бабу
источник