A static field has been lazy initialized without any synchronization used. This will allow race conditions to occur if the field's getter is called on multiple threads at once.

Bad Practice

public class UnsynchronizedLazyInit {
    private static SomeResource u = null;

    static SomeResource getResource() {
        // There is no synchronization used here...
        if (u == null) {
            u = new SomeResource();
        }

        return u;
    }
}

Here, if two threads were to call getResource() while u were null, both threads would attempt to assign a new SomeResource instance to u. In this scenario, one of the threads is likely to overwrite the value of u set in the other thread.

Recommended

There are a number of ways to solve this issue.

Use a synchronized method

This solution may be problematic if some other code also synchronizes on this when a synchronized method is called.

static synchronized SomeResource getResource() {
    if (u == null) {
        u = new SomeResource();
    }

    return u;
}

Synchronize on a private lock variable

This method is safer, since we are now using a private and final value which cannot be locked on directly by external code.

private final Object LOCK = new Object();

static SomeResource getResource() {
    synchronized(LOCK) {
        if (u == null) {
            u = new SomeResource();
        }

        return u;
    }
}

References

CWE-362 - Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition')
CWE-609 - Double-Checked Locking

Glowman554 / BetterNudel

No ignore rules found