TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix double check locking

This commit is contained in:
Oliver Burn 2012-04-16 21:40:11 +10:00
parent b30ed9fc87
commit cbc054705a
6 changed files with 5 additions and 286 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/DoubleCheckedLockingCheck.java
View File

@ -1,93 +0,0 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2011 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.coding;
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 <a href=
* "http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html"
* >The &quot;Double-Checked Locking is Broken&quot; Declaration</a> for a
* more in depth explanation.
*
* @author Lars K&uuml;hne
*/
public class DoubleCheckedLockingCheck extends Check
{
@Override
public int[] getDefaultTokens()
{
return new int[]{TokenTypes.LITERAL_IF};
}
@Override
public void visitToken(DetailAST aAST)
{
final DetailAST synchronizedAST =
getLowestParent(aAST, TokenTypes.LITERAL_SYNCHRONIZED);
if (synchronizedAST == null) {
return;
}
final 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;
}
}

77
src/testinputs/com/puppycrawl/tools/checkstyle/InputDoubleCheckedLocking.java
View File

@ -1,77 +0,0 @@
////////////////////////////////////////////////////////////////////////////////
// Test case file for checkstyle.
// Created: 2001
////////////////////////////////////////////////////////////////////////////////
package com.puppycrawl.tools.checkstyle;
/**
* Test case for detection of double checked locking
* @author lkuehne
**/
class InputDoubleCheckedLocking
{
static Integer one = null;
private static Integer getOneCorrect()
{
synchronized (InputDoubleCheckedLocking.class)
{
if (one == null)
{
one = new Integer(1);
}
}
return one;
}
private static Integer getOneDCL()
{
if (one == null)
{
System.out.println("just to make the AST interesting");
synchronized (InputDoubleCheckedLocking.class)
{
if (one == null)
{
one = new Integer(1);
}
}
}
return one;
}
private static Integer getSimilarToDCL()
{
// different tests
if (one == null)
{
synchronized (InputDoubleCheckedLocking.class)
{
if (one == Integer.valueOf(2))
{
one = new Integer(1);
}
}
}
// no synchronization
if (one == null)
{
if (one == null)
{
one = new Integer(1);
}
}
// no outer test
synchronized (InputDoubleCheckedLocking.class)
{
if (one == null)
{
one = new Integer(1);
}
}
return one;
}
}

42
src/tests/com/puppycrawl/tools/checkstyle/checks/coding/DoubleCheckedLockingCheckTest.java
View File

@ -1,42 +0,0 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2011 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.coding;
import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
import org.junit.Test;
/**
* @author lkuehne
*/
public class DoubleCheckedLockingCheckTest
extends BaseCheckTestSupport
{
@Test
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);
}
}

8
src/xdocs/availablechecks.xml
View File

@ -118,14 +118,6 @@
<td><a href="config_design.html#DesignForExtension">DesignForExtension</a></td>
<td>Checks that classes are designed for inheritance.</td>
</tr>
<tr>
<td><a href="config_coding.html#DoubleCheckedLocking">DoubleCheckedLocking</a></td>
<td>
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.
</td>
</tr>
<tr>
<td><a href="config_blocks.html#EmptyBlock">EmptyBlock</a></td>
<td>Checks for empty blocks.</td>

65
src/xdocs/config_coding.xml
View File

@ -133,71 +133,6 @@ String b = (a==null || a.length&lt;1) ? null : a.substring(1);
</subsection>
</section>
<section name="DoubleCheckedLocking">
<subsection name="Description">
<p>
The &quot;double-checked locking&quot; idiom (DCL) tries to avoid
the runtime cost of synchronization. An example that uses the DCL
idiom is this:
</p>
<source>
public class MySingleton
{
private static theInstance = null;
private MySingleton() {}
public MySingleton getInstance() {
if ( theInstance == null ) { // synchronize only if necessary
synchronized( MySingleton.class ) {
if ( theInstance == null ) {
theInstance = new MySingleton();
}
}
}
}
}
</source>
<p>
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 <a
href="http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html">
&quot;Double-Checked Locking is Broken&quot; Declaration</a> has an
in depth explanation of the exact problem which has to do with the
semantics of the Java memory model.
</p>
<p>
The DoubleCheckedLocking check will find source code where a test is
wrapped in a synchronized block that is wrapped in the same test,
like in the example above.
</p>
</subsection>
<subsection name="Examples">
<p>
To configure the check:
</p>
<source>
&lt;module name=&quot;DoubleCheckedLocking&quot;/&gt;
</source>
</subsection>
<subsection name="Package">
<p>
com.puppycrawl.tools.checkstyle.checks.coding
</p>
</subsection>
<subsection name="Parent Module">
<p>
<a href="config.html#TreeWalker">TreeWalker</a>
</p>
</subsection>
</section>
<section name="EmptyStatement">
<subsection name="Description">
<p>

6
src/xdocs/releasenotes.xml
View File

@ -35,7 +35,11 @@
<p>Notes:</p>
<ul>
<li>
...
Removed the <code>DoubleCheckedLocking</code> check, as in Java 5
(and beyond), using the <code>volatile</code> keyword addresses
the issue. See
<a href="http://jeremymanson.blogspot.com.au/2008/05/double-checked-locking.html">here</a>
for more details.
</li>
</ul>
</section>