Многопоточная программа Java, использующая join (), дает неверные результаты при вычислении суммы смежных чисел

Несинхронизированный доступ к общему изменяемому состоянию обычно не работает.

В вашем Calculation calc есть изменяемая переменная long total. Когда вы запускаете потоки:

  CalculatorThread ct1 = new CalculatorThread(start, end/2 , calc);
  CalculatorThread ct2 = new CalculatorThread( (end/2) + 1, end, calc);

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

Вот рабочая версия:

class ParallelSum {
  public static long calcSum(long start, long end) {
      long total = 0;
      for(long i = start; i < end; i++) {
          total += i;
      }
      return total;
  }

  public static class CalculatorThread extends Thread {
     private long result = 0;
     private long start;
     private long end;
     public CalculatorThread(long start, long end) {
         this.start = start;
         this.end = end;
     }
     @Override
     public void run() {
         result = calcSum(start, end);
     }
     public long getResult() {
         return result;
     }
  }

  public static void main(String[] args) throws InterruptedException {
    int start = 1;
    int end = 100000;
    int endExcl = end + 1;

    CalculatorThread ct1 = new CalculatorThread(start, endExcl/2);
    CalculatorThread ct2 = new CalculatorThread(endExcl / 2, endExcl);

    ct1.start();
    ct2.start();

    ct1.join();
    ct2.join();

    System.out.println(ct1.getResult() + ct2.getResult());
  }
}

Выходы:

5000050000

Дополнительное примечание: всегда используйте индексацию диапазонов [inclusive, exclusive). Это значительно снижает вероятность появления единичных ошибок. Кроме того, я заменил класс Calculation методом: с локальными переменными внутри метода ничего не может пойти не так, и чем меньше изменчивое состояние, тем лучше.

total += i не является атомарным (см. этот ответ). У вас есть два потока, изменяющих одно и то же значение total одновременно, и эти два потока мешают друг другу.

Взгляните на AtomicLong (javadoc), в котором есть метод addAndGet, который атомарно добавляет некоторое число:

class Calculation {
    private AtomicLong total = new AtomicLong();
    public void calcSum(long start, long end) {
        long i = start;
        for( ;i<=end; i++) {
            total.addAndGet(i);
        }
    }
    public long getTotal() {
       return total.get();
    }
}

При таком подходе оба потока могут добавлять к total атомарно, поэтому они больше не будут мешать.

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

Простым решением будет создание двух экземпляров Calculation, по одному для каждого CalculatorThread.

Calculation calc1 = new Calculation();
Calculation calc2 = new Calculation();
CalculatorThread ct1 = new CalculatorThread(start, end/2 , calc1);
CalculatorThread ct2 = new CalculatorThread( (end/2) + 1, end, calc2);

Это классический расчет, который можно разбить на независимые части (которые могут выполняться одновременно) и в конце свести к одному результату.

это не самый простой способ добиться своего результата. Если вы хотите вычислить сумму чисел от 1 до N, вы должны использовать эту формулу: сумма от 1 до N = N (N + 1) / 2. Вы столкнулись с так называемым непоследовательным чтением. Ваши два потока используют одно и то же поле «calc» и особенно длинное общее поле класса Calculation. С вашим решением это может произойти:

trhead1 читает в сумме 0, thread1 увеличивает значение total до 1, thread2 читает в сумме 0, thread2 увеличивает значение total до 1.

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

Основная проблема с вашим кодом заключается в том, что Calculation фактически является оболочкой для общей переменной (поле count), которая обновляется без какой-либо синхронизации.

Есть три способа решить эту проблему:

• синхронизировать переменную; например используя synchronized в соответствующих точках
• использовать атомарный тип для переменной
• не используйте общую переменную.

Третий вариант лучше с точки зрения производительности и прост в реализации; см. ответ @ JenniferP.

Обратите внимание, что дополнительная синхронизация не требуется, если вы даете каждому потоку свой собственный объект Calculation. Отношения происходит раньше, связанные с вызовами Thread::start() и Thread::join(), гарантируют, что основной поток видит правильные результаты от дочерних потоков, когда он читает их после соединения.

Как бы то ни было, поточно-ориентированная реализация Calculation, которая будет работать при совместном использовании экземпляра, будет выглядеть так:

  class Calculation {

      private long total = 0;

      public void calcSum(long start, long end) {        
          for (long i = start; i <= end; i++) {
              increment(i);
          }
      }

      public synchronized long getTotal() {
         return total;
      }

      private synchronized void increment(long by) {
         total += by;
      }
  }

Обратите внимание, что вам необходимо синхронизировать как метод получения, так и метод приращения. Альтернативой является использование AtomicLong для представления общей суммы. Но в обоих случаях возникнут значительные накладные расходы из-за использования общей переменной. (В версии с synchronized накладные расходы могут быть чрезмерными из-за конфликта блокировок. Я был бы удивлен, если бы вообще произошло какое-либо ускорение по сравнению с однопоточной версией. Даже если бы вы могли исключить накладные расходы на создание потоков.)