rspec/rules/S2168/java/rule.adoc

== Why is this an issue?

Double-checked locking is the practice of checking a lazy-initialized object's state both before and after a `synchronized` block is entered to determine whether to initialize the object.
In early JVM versions, synchronizing entire methods was not performant, which sometimes caused this practice to be used in its place.


Apart from `float` and `int` types, this practice does not work reliably in a platform-independent manner without additional synchronization of mutable instances.
Using double-checked locking for the lazy initialization of any other type of primitive or mutable object risks a second thread using an uninitialized or partially initialized member while the first thread is still creating it.
The results can be unexpected, potentially even causing the application to crash.


== How to fix it

Given significant performance improvements of `synchronized` methods in recent JVM versions, `synchronized` methods are now preferred over the less robust double-checked locking.

If marking the entire method as `synchronized` is not an option, consider using an inner `static class` to hold the reference instead.
Inner static classes are guaranteed to be initialized lazily.

=== Code examples

==== Noncompliant code example

[source,java,diff-id=1,diff-type=noncompliant]
----
public class ResourceFactory {
    private static Resource resource;

    public static Resource getInstance() {
        if (resource == null) {
            synchronized (DoubleCheckedLocking.class) { // Noncompliant, not thread-safe due to the use of double-checked locking.
                if (resource == null)
                    resource = new Resource();
            }
        }
        return resource;
    }
}
----


==== Compliant solution

[source,java,diff-id=1,diff-type=compliant]
----
public class ResourceFactory {
    private static Resource resource;

    public static synchronized Resource getInstance() { // Compliant, the entire method is synchronized and hence thread-safe
        if (resource == null)
            resource = new Resource();
        return resource;
    }
}
----

==== Compliant solution

Alternatively, a static inner class can be used.
However, this solution is less explicit in its intention and hence should be used with care.

[source,java]
----
public class ResourceFactory {
    private static class ResourceHolder {
        public static Resource resource = new Resource(); // Compliant, as this will be lazily initialised by the JVM
    }

    public static Resource getResource() {
        return ResourceFactory.ResourceHolder.resource;
    }
}
----

== Resources

* https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html[The "Double-Checked Locking is Broken" Declaration]
* https://wiki.sei.cmu.edu/confluence/x/6zdGBQ[CERT, LCK10-J.] - Use a correct form of the double-checked locking idiom
* CWE - https://cwe.mitre.org/data/definitions/609[CWE-609 - Double-checked locking]
* https://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.4[JLS 12.4] - Initialization of Classes and Interfaces
* Wikipedia: https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java[Double-checked locking]


ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Remove this dangerous instance of double-checked locking.


'''
== Comments And Links
(visible only on this page)

=== on 21 Nov 2024, 16:48:00 Alban Auzeill wrote:
[test-code-support-investigation-for-java] Decision for scope: Main -> All.

=== on 20 Jul 2015, 07:45:24 Ann Campbell wrote:
Tagged java-top by Ann

=== on 8 Nov 2016, 14:58:08 Tibor Blenessy wrote:
I believe that this rule is actually a subset of https://www.securecoding.cert.org/confluence/display/java/TSM03-J.+Do+not+publish+partially+initialized+objects[TSM03-J]. Do we have a rule targeting that? Do we want to implement both?


This rule can be implemented on semantic level, however it will catch only simple cases of this. To do this properly we need to do full escape analysis and implement equivalent of  https://www.securecoding.cert.org/confluence/display/java/TSM03-J.+Do+not+publish+partially+initialized+objects[TSM03-J]

=== on 8 Nov 2016, 16:56:41 Ann Campbell wrote:
\[~tibor.blenessy] I considered adding that mapping to this rule, but really see it as tangential to the rule as currently described. Let me know if you disagree.

=== on 8 Nov 2016, 18:41:56 Tibor Blenessy wrote:
Code samples are from book  Java Concurrency in Practice and they are available under public domain on this url  \http://jcip.net.s3-website-us-east-1.amazonaws.com/listings.html

endif::env-github,rspecator-view[]