diff --git a/docs/config_misc.html b/docs/config_misc.html index a92a439c5..957f2275f 100644 --- a/docs/config_misc.html +++ b/docs/config_misc.html @@ -70,6 +70,9 @@
  • FinalParameters
    • +
  • + DoubleCheckedLocking +
    • @@ -787,6 +790,52 @@ public class StringUtils // not final to allow subclassing TreeWalker

    +

    DoubleCheckedLocking

    Description

    +

    + The "double-checked locking" idiom (DCL) tries to avoid the runtime cost + of synchronization. An example that uses the DCL idiom is this: +

    + 
    +public class MySingleton
+{
+    private theInstance = null;
+
+    private MySingleton() {}
+
+    public MySingleton getInstance() {
+        if ( theInstance == null ) { // synchronize only if necessary
+            synchronized( MySingleton.class ) {
+                if ( s_singleton == null ) {
+                    theInstance = new MySingleton();
+                }
+            }
+        }
+    }
+}
+
    +

    + The problem with the DCL idiom in Java is that it just does not work correctly. + Using it introduces bugs that are extremely hard to track down and reproduce. + The + "Double-Checked Locking is Broken" Declaration has an in depth explanation + of the exact problem which has to do with the semantics of the Java memory model. +

    +

    Examples

    +

    + To configure the check: +

    + 
    +<module name="DoubleCheckedLocking"/>
+
    +

    Package

    +

    + com.puppycrawl.tools.checkstyle.checks +

    +

    Parent Module

    +

    + TreeWalker +

    + diff --git a/docs/releasenotes.html b/docs/releasenotes.html index 6a049cd8d..532f111ed 100644 --- a/docs/releasenotes.html +++ b/docs/releasenotes.html @@ -76,6 +76,9 @@ [Bloch, Effective Java], Item 17 "Use interfaces only to define types"). +
  • Added check to detect the double-checked locking + idiom (module DoubleCheckedLocking, request 709333).
    • +
  • Added check to enforce C style (char c[]) or Java style (char[] c) for array type declaration (module ArrayTypeStyle, request 493380).
    • diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DoubleCheckedLockingCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DoubleCheckedLockingCheck.java new file mode 100644 index 000000000..2050477fd --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DoubleCheckedLockingCheck.java @@ -0,0 +1,93 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 2.1 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +//////////////////////////////////////////////////////////////////////////////// +package com.puppycrawl.tools.checkstyle.checks; + +import antlr.collections.AST; +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; + +/** + * Detect the double-checked locking idiom, a technique that tries to avoid + * synchronization overhead but is incorrect because of subtle artifacts of + * the java memory model. + * + * See The "Double-Checked Locking is Broken" Declaration + * for a more in depth explanation. + * + * @author Lars Kühne + */ +public class DoubleCheckedLockingCheck extends Check +{ + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[]{TokenTypes.LITERAL_IF}; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + DetailAST synchronizedAST = + getLowestParent(aAST, TokenTypes.LITERAL_SYNCHRONIZED); + if (synchronizedAST == null) { + return; + } + + DetailAST ifAST = + getLowestParent(synchronizedAST, TokenTypes.LITERAL_IF); + if (ifAST == null) { + return; + } + + if (getIfCondition(aAST).equalsTree(getIfCondition(ifAST))) { + log(aAST.getLineNo(), aAST.getColumnNo(), + "doublechecked.locking.avoid"); + } + } + + /** + * returns the condition of an if statement. + * @param aIfAST the LITERAL_IF AST + * @return the AST that represents the condition of the if statement + */ + private AST getIfCondition(DetailAST aIfAST) + { + return aIfAST.getFirstChild().getNextSibling(); + } + + /** + * searches towards the root of the AST for a specific AST type. + * @param aAST the starting node for searching (inclusive) + * @param aTokenType the token type to search for + * @return the first token of type aTokenTye or null if no such token exists + */ + private DetailAST getLowestParent(DetailAST aAST, int aTokenType) + { + DetailAST synchronizedParent = aAST; + while (synchronizedParent != null + && synchronizedParent.getType() != aTokenType) + { + synchronizedParent = synchronizedParent.getParent(); + } + return synchronizedParent; + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties index 0d6ce9176..7085fb237 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties @@ -72,3 +72,5 @@ translation.missingKey=Key ''{0}'' missing. simplify.expression=Expression can be simplified. simplify.boolreturn=Conditional logic can be removed. + +doublechecked.locking.avoid=The double-checked locking idiom is broken and should be avoided. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_de.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_de.properties index 0059dba74..5f4a9f43f 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_de.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_de.properties @@ -73,3 +73,5 @@ translation.missingKey= simplify.expression=Der Ausdruck kann vereinfacht werden. simplify.boolreturn=Die Verzweigung sollte entfernt werden. + +doublechecked.locking.avoid=Das ''double-checked locking'' Idiom sollte vermieden werden. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fi.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fi.properties index 92acb54bf..9b4910419 100755 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fi.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fi.properties @@ -72,3 +72,5 @@ translation.missingKey=Käännösavain ''{0}'' puuttuu. simplify.expression=Ilmaisua voisi yksinkertaistaa. simplify.boolreturn=Konditionaalilogiikan voisi poistaa. + +doublechecked.locking.avoid=The double-checked locking idiom is broken and should be avoided. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fr.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fr.properties index 2d2bfec23..255932208 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fr.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_fr.properties @@ -72,3 +72,5 @@ translation.missingKey=Clef ''{0}'' manquante. simplify.expression=L'expression peut-être simplifiée. simplify.boolreturn=Supprimez la logique conditionnelle. + +doublechecked.locking.avoid=The double-checked locking idiom is broken and should be avoided. diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_pt.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_pt.properties index c45e52994..a30c238ef 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_pt.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages_pt.properties @@ -72,3 +72,5 @@ translation.missingKey=Falta a chave ''{0}''. simplify.expression=Expressão pode ser simplicada. simplify.boolreturn=A lógica condicional deve ser removida. + +doublechecked.locking.avoid=The double-checked locking idiom is broken and should be avoided. diff --git a/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java b/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java index 074afc58c..cf09230f4 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java @@ -55,6 +55,7 @@ import com.puppycrawl.tools.checkstyle.checks.UpperEllCheckTest; import com.puppycrawl.tools.checkstyle.checks.VisibilityModifierCheckTest; import com.puppycrawl.tools.checkstyle.checks.WhitespaceAfterCheckTest; import com.puppycrawl.tools.checkstyle.checks.WhitespaceAroundTest; +import com.puppycrawl.tools.checkstyle.checks.DoubleCheckedLockingCheckTest; import junit.framework.Test; import junit.framework.TestSuite; @@ -78,6 +79,7 @@ public class AllTests { suite.addTest(new TestSuite(AvoidStarImportTest.class)); suite.addTest(new TestSuite(ConfigurationLoaderTest.class)); suite.addTest(new TestSuite(ConstantNameCheckTest.class)); + suite.addTest(new TestSuite(DoubleCheckedLockingCheckTest.class)); suite.addTest(new TestSuite(EmptyBlockCheckTest.class)); suite.addTest(new TestSuite(EmptyForIteratorPadCheckTest.class)); suite.addTest(new TestSuite(EqualsHashCodeCheckTest.class)); diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/DoubleCheckedLockingCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/DoubleCheckedLockingCheckTest.java new file mode 100644 index 000000000..560c240df --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/DoubleCheckedLockingCheckTest.java @@ -0,0 +1,22 @@ +package com.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.BaseCheckTestCase; +import com.puppycrawl.tools.checkstyle.DefaultConfiguration; + +/** + * @author lkuehne + */ +public class DoubleCheckedLockingCheckTest + extends BaseCheckTestCase +{ + public void testIt() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(DoubleCheckedLockingCheck.class); + final String[] expected = { + "34:17: The double-checked locking idiom is broken and should be avoided.", + }; + verify(checkConfig, getPath("InputDoubleCheckedLocking.java"), expected); + + } +}