2013-04-23 3 views
195

Чувствительная операция в моей лаборатории сегодня прошла совершенно неправильно. Привод на электронном микроскопе прошел через его границу, и после цепочки событий я потерял 12 миллионов долларов оборудования. Я сужен более 40K строк в неисправном модуле к следующему:Почему эта Java-программа завершается, несмотря на то, что, по-видимому, она не должна (а не)?

import java.util.*; 

class A { 
    static Point currentPos = new Point(1,2); 
    static class Point { 
     int x; 
     int y; 
     Point(int x, int y) { 
      this.x = x; 
      this.y = y; 
     } 
    } 
    public static void main(String[] args) { 
     new Thread() { 
      void f(Point p) { 
       synchronized(this) {} 
       if (p.x+1 != p.y) { 
        System.out.println(p.x+" "+p.y); 
        System.exit(1); 
       } 
      } 
      @Override 
      public void run() { 
       while (currentPos == null); 
       while (true) 
        f(currentPos); 
      } 
     }.start(); 
     while (true) 
      currentPos = new Point(currentPos.x+1, currentPos.y+1); 
    } 
} 

Некоторых образцы продукции я получаю:

$ java A 
145281 145282 
$ java A 
141373 141374 
$ java A 
49251 49252 
$ java A 
47007 47008 
$ java A 
47427 47428 
$ java A 
154800 154801 
$ java A 
34822 34823 
$ java A 
127271 127272 
$ java A 
63650 63651 

Поскольку нет никакой арифметики с плавающей точкой здесь, и все мы знаем, что знаковые целые числа хорошо себя ведут при переполнении в Java, я думаю, что нет ничего плохого в этом коде. Однако, несмотря на вывод, указывающий на то, что программа не достигла условия выхода, она достигла условия выхода (достигнута ли она как и не достигнута?). Зачем?


Я заметил, что этого не происходит в некоторых условиях. Я нахожусь на OpenJDK 6 на 64-битном Linux.

+39

12 milion оборудования? мне действительно интересно, как это могло произойти ... почему вы используете пустой блок синхронизации: synchronized (this) {}? –

+83

Это даже не удаленная по потоку. –

+1

@MattBall, и это уменьшенный код. Очевидно, что запись в 'currentPos' не« слушается »перед чтением, но я не вижу, как это может быть проблемой. – Dog

ответ

135

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

currentPos = new Point(currentPos.x+1, currentPos.y+1); делает несколько вещей, в том числе записи значений по умолчанию для x и y (0), а затем записывают их начальные значения в конструкторе. Поскольку ваш объект не опубликован безопасно, эти 4 операции записи могут быть свободно переупорядочены компилятором/JVM.

Таким образом, с точки зрения читающей нити, это законное исполнение для чтения x с его новым значением, но y с его значением по умолчанию 0. К тому времени, когда вы достигнете оператора println (который, кстати, синхронизирован и, следовательно, влияет на операции чтения), переменные имеют свои начальные значения, и программа печатает ожидаемые значения.

Маркировка currentPos как volatile обеспечит безопасную публикацию, так как ваш объект эффективно неизменен - ​​если в вашем реальном случае использования объекта мутировал после строительства, volatile гарантии не будет достаточно, и вы могли видеть противоречивый объект снова.

В качестве альтернативы вы можете сделать неизменным неизменный Point, который также обеспечит безопасную публикацию, даже без использования volatile. Чтобы достичь неизменности, вам просто нужно отметить x и y final.

В качестве примечания стороны и как уже упоминалось, synchronized(this) {} может рассматриваться как нет-op JVM (я понимаю, вы включили его для воспроизведения поведения).

+4

Я не уверен, но не сделал бы окончание x и y таким же эффектом, избегая барьера памяти? –

+3

Простейший дизайн - неизменный точечный объект, который проверяет инварианты при построении. Поэтому вы никогда не рискуете опубликовать опасную конфигурацию. – Ron

+0

@BuddyCasino Да, действительно, я добавил это. Честно говоря, я не помню всей дискуссии 3 месяца назад (с использованием финала было предложено в комментариях, поэтому не уверен, почему я не включил его в качестве опции). – assylias

29

С currentPos, изменяется вне нити должно быть помечена как volatile:

static volatile Point currentPos = new Point(1,2); 

Без летучий поток не гарантируется читать обновления currentPos, которые делаются в основном потоке. Поэтому новые значения продолжают записываться для currentPos, но поток продолжает использовать предыдущие кешированные версии по соображениям производительности. Поскольку только один поток изменяет currentPos, вы можете уйти без блокировок, что улучшит производительность.

Результаты выглядят совсем по-другому, если вы читаете значения только один раз в потоке для использования при сравнении и последующем их отображении. Когда я делаю следующее, x всегда отображается как 1, а y варьируется от 0 и некоторого большого целого. Я думаю, что поведение этого в этой точке несколько неопределено без ключевого слова volatile, и вполне возможно, что компиляция кода JIT кода вносит свой вклад в это, действуя следующим образом. Кроме того, если я прокомментирую пустой блок synchronized(this) {}, тогда код работает, и я подозреваю, что это связано с тем, что блокировка вызывает достаточную задержку, которая currentPos и ее поля перечитываются, а не используются из кеша.

int x = p.x + 1; 
int y = p.y; 

if (x != y) { 
    System.out.println(x+" "+y); 
    System.exit(1); 
} 
+2

Да, и я мог бы просто поставить замок вокруг всего. В чем ваш смысл? – Dog

+0

Я добавил несколько дополнительных объяснений для использования 'volatile'. –

19

У вас есть обычная память, ссылка «currentpos» и объект Point и его поля за ней, разделенные между двумя потоками без синхронизации. Таким образом, не существует определенного порядка между сообщениями, которые происходят с этой памятью в основном потоке, и чтениями в созданном потоке (назовите его T).

Основной поток делает следующие операции записи (игнорирующие начальной настройки точки, приведет к рх, ру, имеющие значения по умолчанию):

  • РХ
  • ПЙ
  • к currentpos

Поскольку нет ничего особенного в этих сообщениях с точки зрения синхронизации/барьеров, среда выполнения позволяет разрешить поток T видеть их в любом порядке (основной поток, конечно, alw AYS видит запись и чтение упорядочивается по заказу программы), и происходит в любой точке между чтениями в Т.

Так T делают:

  1. читает currentpos к р
  2. чтения точек и р (в любом порядке)
  3. сравнить, и возьмите ветку
  4. чтения точек и ру (или порядок) и вызовите System.out.println

Учитывая, что между основными принципами записи и чтением в T нет упорядочивающих отношений, существует несколько способов, которые могут привести к вашему результату, поскольку T может видеть запись основного файла в currentpos до записей в currentpos.y или currentpos.x :

  1. Он сначала считывает currentpos.x, прежде чем произойдет запись x - получает 0, затем читает currentpos.y до того, как возникла запись y - получает 0. Сравните evals с true. Запись становится видимой для T. System.out.println.
  2. Сначала он считывает currentpos.x, после того, как запись x произошла, затем читает currentpos.y до того, как произошла запись y - получает 0. Сравните evals с true. Записи становятся видимыми для T ... и т. Д.
  3. Он читает currentpos.y во-первых, до того, как произошла запись y (0), затем читает currentpos.x после записи x, evals в true. и т.д.

и так далее ...Здесь есть несколько гонок данных.

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

currentPos = new Point(currentPos.x+1, currentPos.y+1); 

Java не дает такой гарантии (это было бы ужасно для производительности). Что-то еще нужно добавить, если ваша программа нуждается в гарантированном порядке записи относительно чтения в других потоках. Другие предложили сделать окончание x, y окончательным или, альтернативно, сделать текущую ситуацию неустойчивой.

  • Если вы сделаете окончательные поля x, y, то Java гарантирует, что записи их значений будут видны до появления конструктора во всех потоках. Таким образом, поскольку назначение currentpos после конструктора, T-поток гарантированно увидит записи в правильном порядке.
  • Если вы делаете currentpos volatile, тогда Java гарантирует, что это точка синхронизации, которая будет тотально упорядочена по другим точкам синхронизации. Как и в основном, записи в x и y должны происходить до записи в currentpos, тогда любое чтение currentpos в другом потоке должно также видеть записи x, y, которые были ранее.

Использование final имеет то преимущество, что оно делает неизменным поля и, таким образом, позволяет кешировать значения. Использование volatile приводит к синхронизации при каждой записи и чтении currentpos, что может повредить производительность.

Смотри главу 17 из языка Java Spec для окровавленных деталей:. http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html

(Начальный ответ предполагается более слабую модель памяти, так как я не был уверен, что JLS гарантировано летучий было достаточным ответом отредактированных, чтобы отразить комментарий от assylias , указывая на то, что модель Java сильнее - бывает - раньше транзитивна - и поэтому достаточно волатильность на currentpos).

+2

Это лучшее объяснение, на мой взгляд. Большое спасибо! – skyde

+1

@skyde, но неправильно в семантике изменчивости. волатильные гарантии, которые читают изменчивую переменную, будут видеть последнюю доступную запись изменчивой переменной *, а также любую предыдущую запись *. В этом случае, если 'currentPos' становится изменчивым, присваивание обеспечивает безопасную публикацию объекта currentPos, а также его членов, даже если они не являются неустойчивыми. – assylias

+0

Ну, я говорил, что не мог, для себя, точно видеть, как JLS гарантировал, что летучие составляли барьер с другими нормальными чтениями и написаниями. Технически я не могу ошибаться в этом;). Когда дело доходит до моделей памяти, разумно предположить, что заказ не гарантирован и ошибочен (вы все еще в безопасности), чем наоборот, и ошибаетесь и небезопасны. Это здорово, если волатильность обеспечивает эту гарантию. Можете ли вы объяснить, как это делает ch 17 из JLS? – paulj

-3

Вы получаете доступ к currentPos дважды и не гарантируете, что он не обновляется между этими двумя обращениями.

Например:

  1. х = 10, у = 11
  2. рабочий поток оценивает точек, как 10
  3. основной поток выполняет обновление, теперь х = 11 и Y = 12
  4. работника thread оценивает py как 12
  5. рабочий поток отмечает, что 10 + 1! = 12, поэтому печатает и завершает работу.

Вы по существу сравниваете два разных Очков.

Обратите внимание, что даже включение currentPos volatile не защитит вас от этого, так как это два отдельных чтения рабочего потока.

Добавить метод

boolean IsValid() { return x+1 == y; } 

к классу точек. Это гарантирует, что при проверке x + 1 == y используется только одно значение currentPos.

+0

currentPos считывается только один раз, его значение копируется в p. p читается дважды, но он всегда будет указывать на одно и то же место. –

-2

Вы можете использовать объект для синхронизации записей и чтения.В противном случае, как говорили другие, запись в currentPos будет происходить в середине двух прочтений p.x + 1 и p.y.

new Thread() { 
    void f(Point p) { 
     if (p.x+1 != p.y) { 
      System.out.println(p.x+" "+p.y); 
      System.exit(1); 
     } 
    } 
    @Override 
    public void run() { 
     while (currentPos == null); 
     while (true) 
      f(currentPos); 
    } 
}.start(); 
Object sem = new Object(); 
while (true) { 
    synchronized(sem) { 
     currentPos = new Point(currentPos.x+1, currentPos.y+1); 
    } 
} 
+2

Не поможет, если чтение не находится внутри замка. –

+0

На самом деле это делает работу. В моей первой попытке я поместил чтение внутри синхронизированного блока, но позже понял, что это действительно не нужно. –

+1

-1 JVM может доказать, что 'sem' не используется и обрабатывает синхронизированный оператор как нет-op ... Тот факт, что он решает проблему, - это просто удача. – assylias