[GitHub] [kafka] divijvaidya commented on a diff in pull request #12277: KAFKA-13971:Atomicity violations caused by improper usage of ConcurrentHashMap

[GitBox <gi...@apache.org>]

divijvaidya commented on code in PR #12277:
URL: https://github.com/apache/kafka/pull/12277#discussion_r894295650


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfigTransformer.java:
##########
@@ -81,7 +81,7 @@ private void scheduleReload(String connectorName, String path, long ttl) {
         Map<String, HerderRequest> connectorRequests = requests.get(connectorName);
         if (connectorRequests == null) {
             connectorRequests = new ConcurrentHashMap<>();
-            requests.put(connectorName, connectorRequests);
+            requests.putIfAbsent(connectorName, connectorRequests);

Review Comment:
   This is still incorrect. Consider the following scenario:
   
   Thread T1 arrives and reaches line 83. Context switch occurs to thread T2.
   T2 arrives and also reaches line 83. Context switch occurs to thread T1.
   T1 executes line 99 and adds an entry. Context switch occurs to thread T2.
   In a world where we want serialized execution, T2 should have gone to the else block, cancelled the previous request and added the new request but now, T2 will simply overwrite the request at line 99 without having cancelled the previous one.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org