Параллельная задача с синхронизированным arraylist

Question

Моя домашняя работа заключалась в том, чтобы создать проект с использованием распараллеливания, чтобы все было правильно. Тем не менее, я сделал свой проект, но мой специалист сказал, что в моем коде что-то не так. «Пожалуйста, посмотрите список массивов, что-то не в порядке, может быть, синхронизация?».

Я хотел бы попросить вас сообщество помочь мне и указать, что может быть неправильным. Я думаю, что это может быть проблемой, если не покрыть synchronize скобки мой список массива, я прав?

import java.util.ArrayList; 
import java.util.Collections; 
import java.util.List; 
import java.util.logging.Level; 
import java.util.logging.Logger; 

    /** 
    * My project finds all dividors for specific number 
    *It must use threads, so I made them. First I start them (first loop) 
    *then join them (second loop). My project must have that loops. 
    *Problem might be with not synchronizing methods array list... 
    */ 

    public class Main { 

     private final static int NUMBER = 100; 
     private final static List<Integer> dividors = new ArrayList<Integer>(); 

     public static void main(String[] args) { 
      new Main().doStuff(); 
     } 
     private int sqr; 
     private int sqrp1; 

     private void doStuff() { 

      sqr = (int) Math.sqrt(NUMBER); 
      sqrp1 = sqr + 1; 

      Thread[] t = new Thread[sqrp1]; 

     //starting tasks 
      for (int i = 1; i < sqrp1; i++) { 
       final int it = i; 

       if (NUMBER % i == 0) { 
        final int e = i; 

        t[i] = new Thread(new Runnable() { 
         @Override 
         public void run() { 
          System.out.println("sta"+e); 
          if (!checkContains(e)) { 
           addElement(e); 
          } 

          final int dividednumber = NUMBER/e; 

          if (!checkContains(dividednumber)) { 
           addElement(dividednumber); 
          } 
         } 
        }); 
        t[i].start(); 
       } 
      } 

     //calling join for tasks 
      for (int i = 1; i < sqrp1; i++) { 
       final int it = i; 

       if (NUMBER % i == 0) { 
        try { 
         System.out.println("sto"+i); 
         t[i].join(); 
        } catch (InterruptedException ex) { 
         Logger.getLogger(Main.class.getName()).log(Level.SEVERE, null, ex); 
        } 
       } 
      } 


      System.out.println("xxx"); 

      Collections.sort(dividors); 
      Integer[] arrayDividors = dividors.toArray(new Integer[0]); 

      for (int i = 0; i < arrayDividors.length; i++) { 
       System.out.println(arrayDividors[i]); 
      } 
     } 

     private synchronized void addElement(int element) { 
      dividors.add(element); 
     } 

     private synchronized boolean checkContains(int element) { 
      return dividors.contains(element); 
     } 
    } 

---

Правильно ли я изменить эту часть, это нормально сейчас?

t[i] = new Thread(new Runnable() { 
    @Override 
    public void run() { 
     System.out.println("waiting " + e); 
     synchronized (this) { 
      System.out.println("entering " + e); 

      if (!checkContains(e)) { 
       addElement(e); 
      } 

      final int dividednumber = NUMBER/e; 

      if (!checkContains(dividednumber)) { 
       addElement(dividednumber); 
      } 
      System.out.println("leaving " + e); 
     } 
    } 
}); 

Answer 1

Вам необходимо превратить это в одну атомную операцию.

if (!checkContains(dividednumber)) { 
    addElement(dividednumber); 
} 

Представьте, что у вас есть две темы.

T1: if (!checkContains(dividednumber)) { // false 
T2: if (!checkContains(dividednumber)) { // false 
T1:  addElement(dividednumber); // adds number 
T2:  addElement(dividednumber); // adds same number 

Если у вас есть один addElementWithoutDuplicates, этого не произойдет.