Синхронизированное ключевое слово не работает

Question

package threadShareResource1; 

public class NonSynchro1 { 
    private int sum = 0; 

    public static void main(String[] args) { 
     NonSynchro1 n = new NonSynchro1(); 
     n.task(); 
     System.out.println(n.getSum()); 
    } 

    public synchronized void sumAddOne(){ 
     sum++; 
    } 

    public void task(){ 
     for (int i = 0; i < 100; i++) { 
      new Thread(new Runnable(){ 
       @Override 
       public void run() { 
        sumAddOne(); 
       } 
      }).start(); 

     /* try { 
       Thread.sleep(10); 
      } catch (InterruptedException e) { 
       e.printStackTrace(); 
      } */ 
     } 
    } 
    public int getSum() { 
     return sum; 
    } 
} 

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

Я также пытаюсь использовать ExecutorService, но он не дает 100 всех прогонов.

public void task(){ 
    ExecutorService s = Executors.newCachedThreadPool(); 

    for (int i = 0; i < 100; i++) { 
     s.execute(new Thread(new Runnable(){ 
      @Override 
      public void run() { 
       sumAddOne(); 
      } 
     })); 
    } 
    s.shutdown(); 

    while(!s.isTerminated()){} 
} 

Answer 1

В Task(), вы начинаете 100 потоков (что много), и каждый из них является добавление 1 к сумме.

Но когда задача выполнена, все, что вы знаете, это то, что в процессе начального процесса запущено 100 потоков. Вы не блокируете перед вызовом println(), так как вы знаете, что все потоки завершены?

Сон, вероятно, «предотвращает коррупцию» только потому, что он дает системному времени завершить запуск всех потоков.

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

Answer 2

Правильное использование синхронизированного ключевого слова, проблема в том, что вы не дожидаетесь завершения потоков. Вот одно из возможных решений:

public class NonSynchro1 { 

    private static final ExecutorService executorService = Executors.newCachedThreadPool(); 

    private int sum = 0; 

    public static void main(String[] args) { 
     NonSynchro1 n = new NonSynchro1(); 
     n.task(); 
     System.out.println(n.getSum()); 
     executorService.shutdown(); 
    } 

    public synchronized void sumAddOne() { 
     sum++; 
    } 

    public void task() { 
     List<Callable<Object>> callables = new ArrayList<>(); 
     for (int i = 0; i < 100; i++) { 
      callables.add(() -> { 
       sumAddOne(); 
       return null; 
      }); 
     } 

     List<Future<Object>> futures; 
     try { 
      futures = executorService.invokeAll(callables); 
     } catch (InterruptedException e) { 
      throw new RuntimeException(e); 
     } 

     futures.forEach(future -> { 
      try { 
       future.get(); 
      } catch (ExecutionException | InterruptedException e) { 
       throw new RuntimeException(e); 
      } 
     }); 
    } 

    public int getSum() { 
     return sum; 
    } 

} 

Сначала мы создаем список вызываемых объектов - список функций, которые будут выполняться параллельно.

Затем мы вызываем их на службу исполнителя. newCachedThreadPool Я использовал здесь, по умолчанию имеет 0 потоков, он будет создавать столько, сколько необходимо для выполнения всех переданных вызовов, потоки будут убиты после простоя в течение минуты.

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

Кроме того, полезно прекратить работу службы-исполнителя, если вы хотите законно завершить программу. В этом случае это просто executorService.shutdown() в конце основного метода. Если вы этого не сделаете, программа завершится через минуту, когда простаивающие потоки будут убиты. Однако, если различная служба-исполнитель, потоки могут не быть убиты при простоях, и в этом случае программа никогда не завершится.

Answer 3

Просто для полноты: Вот решение, показывающее, как исходная программа может быть сделана, чтобы ждать, пока все нити, чтобы закончить joining них:

 for (Thread t : n.task()) 
      try { 
       t.join(); 
      } catch (InterruptedException e) { 
       e.printStackTrace(); 
      } 

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

public class TestSynchro1 { 
    private int sum = 0; 

    public synchronized void sumAddOne() { 
     sum++; 
    } 

    public Thread[] task(int n) { 
     Thread[] threads = new Thread[n]; 
     for (int i = 0; i < n; i++) { 
      (threads[i] = new Thread(new Runnable() { 
       @Override 
       public void run() { 
        sumAddOne(); 
       } 
      })).start(); 
     } 
     return threads; 
    } 

    public static void main(String[] args) { 
     TestSynchro1 n = new TestSynchro1(); 
     for (Thread t : n.task(100)) 
      try { 
       t.join(); 
      } catch (InterruptedException e) { 
       e.printStackTrace(); 
      } 
     System.out.println(n.sum); 
    } 
}