이 코드는 스레드로부터 안전한 것입니까?

Question

클라이언트 요청의 흐름을 처리하려고합니다. 각 요청에는 특별한 유형이 있습니다. 먼저 해당 유형에 대한 일부 데이터를 초기화해야하며 이후에는 요청 처리를 시작할 수 있습니다. 클라이언트 유형이 처음 제공되면 해당 데이터를 초기화합니다. 이 후 해당 유형의 모든 요청은 해당 데이터를 사용하여 처리됩니다.

스레드로부터 안전한 방식으로이 작업을 수행해야합니다.

다음은 내가 작성한 코드입니다. 스레드로부터 안전합니까?

public class Test { 

    private static Map<Integer, Object> clientTypesInitiated = new ConcurrentHashMap<Integer, Object>(); 

    /* to process client request we need to 
    create corresponding client type data. 
    on the first signal we create that data, 
    on the second - we process the request*/ 

    void onClientRequestReceived(int clientTypeIndex) { 
     if (clientTypesInitiated.put(clientTypeIndex, "") == null) { 
      //new client type index arrived, this type was never processed 
      //process data for that client type and put it into the map of types 
      Object clientTypeData = createClientTypeData(clientTypeIndex); 
      clientTypesInitiated.put(clientTypeIndex, clientTypeData); 
     } else { 
      //already existing index - we already have results and we can use them 
      processClientUsingClientTypeData(clientTypesInitiated.get(clientTypeIndex)); 
     } 
    } 

    Object createClientTypeData(int clientIndex) {return new Object();} 

    void processClientUsingClientTypeData(Object clientTypeData) {} 
} 

한 손에서 ConcurrentHashMap의은 map.put (A가 B) == 한편에서 동일한 A. null을 두 번, 과제 및 comparisson 조작 스레드 세이프되지 생산할 수 없다.

코드가 맞습니까? 그렇지 않은 경우 어떻게 수정해야합니까?

업데이트 : Martin Serrano는 코드가 스레드로부터 안전하며 이중 초기화 문제가 발생하지 않으므로 답변을 수락했습니다. 그러나 아래에 답변으로 게시 된 버전과 함께 발행물을 찾지 못했고 내 버전에는 동기화가 필요하지 않습니다.

Answer 1

이 코드는

//already existing index - we already have results and we can use them 
processClientUsingClientTypeData(clientTypesInitiated.get(clientTypeIndex)); 

이 일시적으로 넣어 검사에 삽입 ""값을 얻기의 기회가 안전하기 때문에 스레드되지 않습니다.

public class Test { 

    private static Map<Integer, Object> clientTypesInitiated = new ConcurrentHashMap<Integer, Object>(); 

    /* to process client request we need to 
     create corresponding client type data. 
     on the first signal we create that data, 
     on the second - we process the request*/ 

void onClientRequestReceived(int clientTypeIndex) { 
    Object clientTypeData = clientTypesInitiated.get(clientTypeIndex); 
    if (clientTypeData == null) { 
     synchronized (clientTypesInitiated) { 
      clientTypeData = clientTypesInitiated.get(clientTypeIndex); 
      if (clientTypeData == null) { 
       //new client type index arrived, this type was never processed 
       //process data for that client type and put it into the map of types 
       clientTypeData = createClientTypeData(clientTypeIndex); 
       clientTypesInitiated.put(clientTypeIndex, clientTypeData); 
      } 
     } 
    } 
    processClientUsingClientTypeData(clientTypeData); 
} 

Object createClientTypeData(int clientIndex) {return new Object();} 

void processClientUsingClientTypeData(Object clientTypeData) {} 

}

Answer 2

아니요, 여전히 스레드로부터 안전하다고는 생각하지 않습니다.

동기화 된 블록에 넣기 작업을 래핑해야합니다.

일반적으로 차단하지 않습니다 (GET 포함) javadoc for ConcurrentHashMap

검색 작업 당으로

때문에 갱신 조작과 오버랩하는 경우가 있습니다 (넣어 제거 포함).

Answer 3

없음을 당신이 스레드 안전 할 모든 방법을 필요로하는 경우 동기화로 선언하거나 이 중요한 코드를 삽입에 동기화 된 블록을 사용하지 없습니다.

Answer 4

여기에서 putIfAbsent를 사용해야합니다.이 작업의 의미는 CAS와 유사하며 확실히 원자력입니다. 그리고 원자 적이기 때문에 내부 할당과 비교에 문제가 없습니다.

현재 코드는 스레드로부터 안전하지 않습니다.

Answer 5

if와 next put 사이에서 컨텍스트 전환이 발생할 수 있으므로 현재 코드는 스레드로부터 안전하지 않습니다. clientTypesInitiated을 ConcurrentHashMap 유형으로 지정해야하며 인터페이스 맵은 필요하지 않습니다. 그리고 나서 putIfAbsent 방법을 사용하십시오.

Answer 6

코드가 의도 한대로 작동하지 않는다고 생각합니다. 보세요 :

void onClientRequestReceived(int clientTypeIndex) { 
    // the line below ensures the existing data is lost even if was not null 
    if (clientTypesInitiated.put(clientTypeIndex, "") == null) { 
     Object clientTypeData = createClientTypeData(clientTypeIndex); 
     clientTypesInitiated.put(clientTypeIndex, clientTypeData); 
    } else { 
     processClientUsingClientTypeData(clientTypesInitiated.get(clientTypeIndex)); 
    } 
} 

당신은 오히려 거기에 get 메소드 호출을 원했을 것입니다.

나는 그러한 접근을 건의 할 것입니다 :

void onClientRequestReceived(int clientTypeIndex) { 
    Object clientTypeData; 
    synchronized (clientTypesInitiated) { 
     if ((clientTypeData = clientTypesInitiated.get(clientTypeIndex)) == null) { 
      clientTypeData = createClientTypeData(clientTypeIndex); 
      clientTypesInitiated.put(clientTypeIndex, clientTypeData); 
     } 
    } 
    processClientUsingClientTypeData(clientTypeData); 
} 

Answer 7

적 putIfAbsent 잘 작동하지만, 경우에 따라서는 여러 번 반복 초기화가 필요합니다 :

이 코드

는 thusly 히 스레드 할 수 있습니다. 이중 검사 잠금을 사용하는 동기화 된 블록이이 문제를 해결할 수 있습니다.

public class Test { 

    private static Map<Integer, Object> clientTypesInitiated = new ConcurrentHashMap<Integer, Object>(); 

    private static final int CLIENT_TYPES_NUMBER = 10; 
    private static AtomicIntegerArray clientTypesInitStarted = new AtomicIntegerArray(CLIENT_TYPES_NUMBER); 
    private static AtomicIntegerArray clientTypesInitFinished = new AtomicIntegerArray(CLIENT_TYPES_NUMBER); 

    void onClientRequestReceived(int clientTypeIndex) { 

     int isInitStarted = clientTypesInitStarted.getAndIncrement(clientTypeIndex); 
     int isInitFinished = clientTypesInitFinished.get(clientTypeIndex); 

     if (isInitStarted == 0) { 
      //new client type index arrived, this type was never processed 
      //process data for that client type and put it into the map of types 
      Object clientTypeData = createClientTypeData(clientTypeIndex); 
      clientTypesInitiated.put(clientTypeIndex, clientTypeData); 
      clientTypesInitFinished.getAndIncrement(clientTypeIndex); 
     } else if (isInitFinished > 0) { 
      //already existing index - we already have results and we can use them 
      processClientUsingClientTypeData(clientTypesInitiated.get(clientTypeIndex)); 
     } 
    } 

    Object createClientTypeData(int clientIndex) { 
     return new Object(); 
    } 

    void processClientUsingClientTypeData(Object clientTypeData) { 
    } 
} 

모든 의견 :

그러나 여기가 AtomicIntegerArrays에 따라 다른 옵션을입니까?

Answer 8

ConcurrentHashMap은 동기화 된 블록을 사용하지 않도록 생성되었지만 동기화 된 블록의 잠금으로 사용하는 것은 좋지만 효율적이지는 않습니다. ConcurrentHashMap을 사용해야하는 경우 올바른 사용법은 다음과 같습니다.

private static Map<Integer, Object> clientTypesInitiated = new ConcurrentHashMap<Integer, Object>(); 

void onClientRequestReceived(int clientTypeIndex) { 
    Object clientTypeData = clientTypesInitiated.get(clientTypeIndex); 
    if (clientTypeData == null) { 
     Object newClientTypeData = createClientTypeData(clientTypeIndex); 
     clientTypeData = clientTypesInitiated.putIfAbsent(clientTypeIndex, newClientTypeData); 
     // if clientTypeData is null, then put successful, otherwise, use the old one 
     if (clientTypeData == null) { 
      clientTypeData = newClientTypeData; 
     } 
    } 
    processClientUsingClientTypeData(clientTypeData); 
} 

는 솔직히 말해서,이 synchronized 블록을 사용하지 않지만 (인종 조건) clientTypeData 여러 번 만들 수 있습니다 위의 코드. 따라서 clientTypeData을 만드는 것이 얼마나 비쌉니까 측정해야하며 너무 비싸서 synchronized 블록을 사용하지 않는 경우에는 ConcurrentHashMap을 사용할 필요가 없습니다. 일반 HashMap 및 synchronized 블록을 사용하십시오.