new "non-atomic operation on volatile field" inspection

drop "arithmetic operation on volatile field" inspection
2026-09-27 10:03:11 +07:00 · 2010-11-11 15:57:44 +01:00
parent b6425c6641
commit 8038b8fd86
6 changed files with 143 additions and 118 deletions
@@ -816,7 +816,7 @@ set.replaceable.by.enum.set.display.name=Set replaceable with EnumSet
 non.static.inner.class.in.secure.context.display.name=Non-static inner class in secure context
 tail.recursion.display.name=Tail recursion
 finally.block.cannot.complete.normally.display.name='finally' block which can not complete normally
-arithmetic.on.volatile.field.display.name=Arithmetic operation on volatile field
+non.atomic.operation.on.volatile.field.display.name=Non-atomic operation on volatile field
 public.static.collection.field.display.name='public static' collection field
 non.exception.name.ends.with.exception.display.name=Non-exception class name ends with 'Exception'
 synchronized.method.display.name='synchronized' method
@@ -993,7 +993,7 @@ serializable.inner.class.has.serial.version.uid.field.problem.descriptor=Inner c
 serializable.inner.class.with.non.serializable.outer.class.problem.descriptor=Inner class <code>#ref</code> is serializable while its outer class is not #loc
 busy.wait.problem.descriptor=Call to <code>Thread.#ref()</code> in a loop, probably busy-waiting #loc
 sleep.while.holding.lock.problem.descriptor=Call to <code>Thread.#ref()</code> while synchronized #loc
-arithmetic.on.volatile.field.problem.descriptor=Arithmetic operation on volatile field <code>#ref</code> #loc
+non.atomic.operation.on.volatile.field.problem.descriptor=Non-atomic operation on volatile field <code>#ref</code> #loc
 call.to.native.method.while.locked.problem.descriptor=Call to native method <code>#ref()</code> in a synchronized context #loc
 object.notify.problem.descriptor=<code>#ref</code> should probably be replaced with 'notifyAll()' #loc
 condition.signal.problem.descriptor=<code>#ref</code> should probably be replaced with 'signalAll()' #loc
@@ -821,7 +821,6 @@ public class InspectionGadgetsPlugin implements ApplicationComponent,
    private void registerThreadingInspections() {
        m_inspectionClasses.add(AccessToNonThreadSafeStaticFieldFromInstanceInspection.class);
        m_inspectionClasses.add(AccessToStaticFieldLockedOnInstanceInspection.class);
-        m_inspectionClasses.add(ArithmeticOnVolatileFieldInspection.class);
        m_inspectionClasses.add(AwaitNotInLoopInspection.class);
        m_inspectionClasses.add(AwaitWithoutCorrespondingSignalInspection.class);
        m_inspectionClasses.add(BusyWaitInspection.class);
@@ -834,6 +833,7 @@ public class InspectionGadgetsPlugin implements ApplicationComponent,
        m_inspectionClasses.add(MethodMayBeSynchronizedInspection.class);
        m_inspectionClasses.add(NakedNotifyInspection.class);
        m_inspectionClasses.add(NestedSynchronizedStatementInspection.class);
+        m_inspectionClasses.add(NonAtomicOperationOnVolatileFieldInspection.class);
        m_inspectionClasses.add(NonSynchronizedMethodOverridesSynchronizedMethodInspection.class);
        m_inspectionClasses.add(NotifyCalledOnConditionInspection.class);
        m_inspectionClasses.add(NotifyNotInSynchronizedContextInspection.class);
@@ -1,105 +0,0 @@
-/*
- * Copyright 2003-2007 Dave Griffith, Bas Leijdekkers
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.siyeh.ig.threading;
-
-import com.intellij.psi.*;
-import com.intellij.psi.tree.IElementType;
-import com.siyeh.InspectionGadgetsBundle;
-import com.siyeh.ig.BaseInspection;
-import com.siyeh.ig.BaseInspectionVisitor;
-import com.siyeh.ig.psiutils.WellFormednessUtils;
-import org.jetbrains.annotations.NotNull;
-
-public class ArithmeticOnVolatileFieldInspection extends BaseInspection {
-
-    @NotNull
-    public String getDisplayName() {
-        return InspectionGadgetsBundle.message(
-                "arithmetic.on.volatile.field.display.name");
-    }
-
-    @NotNull
-    protected String buildErrorString(Object... infos) {
-        return InspectionGadgetsBundle.message(
-                "arithmetic.on.volatile.field.problem.descriptor");
-    }
-
-    public BaseInspectionVisitor buildVisitor() {
-        return new AritmeticOnVolatileFieldInspection();
-    }
-
-    private static class AritmeticOnVolatileFieldInspection
-            extends BaseInspectionVisitor {
-
-        @Override public void visitBinaryExpression(
-                @NotNull PsiBinaryExpression expression) {
-            super.visitBinaryExpression(expression);
-            if (expression.getROperand() == null) {
-                return;
-            }
-            final PsiJavaToken sign = expression.getOperationSign();
-            final IElementType tokenType = sign.getTokenType();
-            if (!JavaTokenType.ASTERISK.equals(tokenType) &&
-                    !JavaTokenType.DIV.equals(tokenType) &&
-                    !JavaTokenType.PLUS.equals(tokenType) &&
-                    !JavaTokenType.MINUS.equals(tokenType) &&
-                    !JavaTokenType.PERC.equals(tokenType)) {
-                return;
-            }
-            final PsiExpression lhs = expression.getLOperand();
-            checkForVolatile(lhs);
-            final PsiExpression rhs = expression.getROperand();
-            checkForVolatile(rhs);
-        }
-
-        @Override public void visitAssignmentExpression(
-                @NotNull PsiAssignmentExpression expression) {
-            super.visitAssignmentExpression(expression);
-            if (!WellFormednessUtils.isWellFormed(expression)) {
-                return;
-            }
-            final PsiJavaToken sign = expression.getOperationSign();
-            final IElementType tokenType = sign.getTokenType();
-            if (!JavaTokenType.ASTERISKEQ.equals(tokenType) &&
-                    !JavaTokenType.DIVEQ.equals(tokenType) &&
-                    !JavaTokenType.PLUSEQ.equals(tokenType) &&
-                    !JavaTokenType.MINUSEQ.equals(tokenType) &&
-                    !JavaTokenType.PERCEQ.equals(tokenType)) {
-                return;
-            }
-            final PsiExpression lhs = expression.getLExpression();
-            checkForVolatile(lhs);
-            final PsiExpression rhs = expression.getRExpression();
-            checkForVolatile(rhs);
-        }
-
-        private void checkForVolatile(PsiExpression expression) {
-            if (!(expression instanceof PsiReferenceExpression)) {
-                return;
-            }
-            final PsiReferenceExpression reference =
-                    (PsiReferenceExpression)expression;
-            final PsiElement referent = reference.resolve();
-            if (!(referent instanceof PsiField)) {
-                return;
-            }
-            final PsiField field = (PsiField)referent;
-            if (field.hasModifierProperty(PsiModifier.VOLATILE)) {
-                registerError(expression);
-            }
-        }
-    }
-}
@@ -0,0 +1,127 @@
+/*
+ * Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.siyeh.ig.threading;
+
+import com.intellij.psi.*;
+import com.intellij.psi.tree.IElementType;
+import com.siyeh.InspectionGadgetsBundle;
+import com.siyeh.ig.BaseInspection;
+import com.siyeh.ig.BaseInspectionVisitor;
+import com.siyeh.ig.psiutils.VariableAccessUtils;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+public class NonAtomicOperationOnVolatileFieldInspection
+        extends BaseInspection {
+
+    @Override
+    @NotNull
+    public String getDisplayName() {
+        return InspectionGadgetsBundle.message(
+                "non.atomic.operation.on.volatile.field.display.name");
+    }
+
+    @Override
+    @NotNull
+    protected String buildErrorString(Object... infos) {
+        return InspectionGadgetsBundle.message(
+                "non.atomic.operation.on.volatile.field.problem.descriptor");
+    }
+
+    @Override
+    public BaseInspectionVisitor buildVisitor() {
+        return new NonAtomicOperationOnVolatileFieldVisitor();
+    }
+
+    private static class NonAtomicOperationOnVolatileFieldVisitor
+            extends BaseInspectionVisitor {
+
+        @Override public void visitAssignmentExpression(
+                @NotNull PsiAssignmentExpression expression) {
+            super.visitAssignmentExpression(expression);
+            final PsiExpression rhs = expression.getRExpression();
+            if (rhs == null) {
+                return;
+            }
+            final PsiExpression lhs = expression.getLExpression();
+            final PsiField volatileField = findVolatileField(lhs);
+            if (volatileField ==  null) {
+                return;
+            }
+            final IElementType tokenType = expression.getOperationTokenType();
+            if (tokenType.equals(JavaTokenType.PLUSEQ) ||
+                    tokenType.equals(JavaTokenType.MINUSEQ) ||
+                    tokenType.equals(JavaTokenType.ASTERISKEQ) ||
+                    tokenType.equals(JavaTokenType.DIVEQ) ||
+                    tokenType.equals(JavaTokenType.ANDEQ) ||
+                    tokenType.equals(JavaTokenType.OREQ)||
+                    tokenType.equals(JavaTokenType.XOREQ)||
+                    tokenType.equals(JavaTokenType.PERCEQ)||
+                    tokenType.equals(JavaTokenType.LTLTEQ)||
+                    tokenType.equals(JavaTokenType.GTGTEQ)||
+                    tokenType.equals(JavaTokenType.GTGTGTEQ)) {
+                registerError(lhs);
+                return;
+            }
+            if (VariableAccessUtils.variableIsUsed(volatileField, rhs)) {
+                registerError(lhs);
+            }
+        }
+
+        @Override
+        public void visitPrefixExpression(PsiPrefixExpression expression) {
+            super.visitPrefixExpression(expression);
+            final PsiExpression operand = expression.getOperand();
+            if (operand == null) {
+                return;
+            }
+            final PsiField volatileField = findVolatileField(operand);
+            if (volatileField == null) {
+                return;
+            }
+            registerError(operand);
+        }
+
+        @Override
+        public void visitPostfixExpression(PsiPostfixExpression expression) {
+            super.visitPostfixExpression(expression);
+            final PsiExpression operand = expression.getOperand();
+            final PsiField volatileField = findVolatileField(operand);
+            if (volatileField == null) {
+                return;
+            }
+            registerError(operand);
+        }
+
+        @Nullable
+        private static PsiField findVolatileField(PsiExpression expression) {
+            if (!(expression instanceof PsiReferenceExpression)) {
+                return null;
+            }
+            final PsiReferenceExpression reference =
+                    (PsiReferenceExpression)expression;
+            final PsiElement referent = reference.resolve();
+            if (!(referent instanceof PsiField)) {
+                return null;
+            }
+            final PsiField field = (PsiField)referent;
+            if (!field.hasModifierProperty(PsiModifier.VOLATILE)) {
+                return null;
+            }
+            return field;
+        }
+    }
+}
@@ -1,10 +0,0 @@
-<html>
-<body><table> <tr> <td valign="top" height="150">
-<font face="verdana" size="-1">
-This inspection reports any uses of volatile fields in arithmetic operations.
-It's a common misconception that such operations are effectively atomic, but the
-Java Memory Model only specifies that loads and stores on volatile variables are
-atomic. This can lead to unexpected results, including lost updates, when using
-volatile fields in arithmetic operations.
-</font></td> </tr> <tr> <td height="20"> <font face="verdana" size="-2">Powered by InspectionGadgets </font> </td> </tr> </table> </body>
-</html>
@@ -0,0 +1,13 @@
+<html>
+<body><table> <tr> <td valign="top" height="150">
+<font face="verdana" size="-1">
+This inspection reports any non-atomic operations on volatile fields. Non-atomic
+operations on volatile fields are operations where the volatile field is read and
+the value is used to update the volatile field. It is possible for the value of the
+field to change between the read and write, making the operation possibly invalid.
+In such cases it is better to use one of the <b><font
+        color="#000080">Atomic*</font></b> or
+    <b><font color="#000080">Atomic*FieldUpdater</font></b> classes
+from the <b><font color="#000080">java.util.concurrent.atomic</font></b> package.
+</font></td> </tr> <tr> <td height="20"> <font face="verdana" size="-2">New in 10, Powered by InspectionGadgets</font> </td> </tr> </table> </body>
+</html>