[GitHub] [skywalking] nisiyong commented on a change in pull request #6009: fix: Non-atomic operation on volatile field

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

nisiyong commented on a change in pull request #6009:
URL: https://github.com/apache/skywalking/pull/6009#discussion_r543079362



##########
File path: apm-commons/apm-datacarrier/src/main/java/org/apache/skywalking/apm/commons/datacarrier/consumer/MultipleChannelsConsumer.java
##########
@@ -30,13 +32,12 @@
 public class MultipleChannelsConsumer extends Thread {
     private volatile boolean running;
     private volatile ArrayList<Group> consumeTargets;
-    @SuppressWarnings("NonAtomicVolatileUpdate")
-    private volatile long size;
+    private final AtomicLong size = new AtomicLong();

Review comment:
       Yes, there has another way to fix this warning. Just add `synchronized` on the `MultipleChannelsConsumer#addNewTarget`, but it has already added on `BulkConsumePool#add` which invoke `MultipleChannelsConsumer#addNewTarget`.
   
   IDE shows warning because the volatile filed has non-atomic operations on this Class, the non-atomic operations should be synchronized. Or we just use AtomicLong.




----------------------------------------------------------------
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.

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