False Negative: DoubleCheckedLocking.ql misses non-volatile lazy initialization once the null checks are split across locals, helpers, or early returns.

Inizia da Likely Bugs/Concurrency/DoubleCheckedLocking.ql e confronta la sua logica di matching con PosCase1_Var2.java, PosCase1_Var3.java, PosCase1_Var5.java, PosCase2_Var3.java e PosCase2_Var5.java. Traccia il modo in cui la query modella le letture non volatile, l'inizializzazione sincronizzata, le variabili locali, gli helper e i return anticipati. Il lavoro è completato quando il checker segnala tutti e cinque i pattern non sicuri senza dipendere da un'unica forma canonica dell'AST.

False Negative: DoubleCheckedLocking.ql misses non-volatile lazy initialization once the null checks are split across locals, helpers, or early returns.

Version
codeql 2.24.3

Checker

• Checker id: Likely Bugs/Concurrency/DoubleCheckedLocking.ql
• Checker description: Detects double-checked locking patterns on non-volatile fields that are not thread-safe due to improper synchronization and field visibility issues.

Description of the false negative

All five samples preserve the same unsafe publication pattern: a non-volatile field is read outside synchronization and lazily initialized under synchronization. The code only changes how the null checks and the return are spelled.

That should still be enough for Likely Bugs/Concurrency/DoubleCheckedLocking.ql to report the pattern.

Affected test cases

PosCase1_Var2.java

The boolean temporaries only cache the same two null checks. This is still ordinary double-checked locking on a non-volatile field.

// Double-checked locking on non-volatile, non-immutable field should be flagged as thread-unsafe.
package scensct.var.pos;

public class PosCase1_Var2 {
    private Object instance;

    public Object getInstance() {
        // outer check with boolean flag
        boolean needSync = instance == null;
        if (needSync) {
            synchronized (this) {
                // inner check with explicit condition variable
                boolean stillNull = instance == null;
                if (stillNull) {
                    instance = new Object();
                }
            }
        }
        return instance;
    }
}

PosCase1_Var3.java

The early return does not make the pattern safe. The field is still observed outside synchronization and initialized lazily inside the synchronized block.

// Double-checked locking on non-volatile, non-immutable field should be flagged as thread-unsafe.
package scensct.var.pos;

public class PosCase1_Var3 {
    private Object instance;

    public Object getInstance() {
        if (instance != null) {
            return instance; // early return instead of if-null
        }
        synchronized (this) {
            if (instance == null) {
                instance = new Object();
            }
            return instance; // return inside synchronized block
        }
    }
}

PosCase1_Var5.java

Moving the inner null check into initSingleton() only hides the same unsafe initialization pattern behind a helper.

// Double-checked locking on non-volatile, non-immutable field should be flagged as thread-unsafe.
package scensct.var.pos;

public class PosCase1_Var5 {
    private Object instance;

    private void initSingleton() {
        if (instance == null) {
            instance = new Object();
        }
    }

    public Object getInstance() {
        if (instance == null) {
            synchronized (this) {
                // inner check moved into helper but still inside sync
                initSingleton();
            }
        }
        return instance;
    }
}

PosCase2_Var3.java

The helper methods separate the outer check, the synchronized initialization, and the final read, but the field is still accessed outside synchronization and remains non-volatile.

// Double-checked locking on non-volatile immutable-type field with field access outside synchronized block should be flagged.
package scensct.var.pos;

public class PosCase2_Var3 {
    private String instance;

    public String getInstance() {
        if (isNotInitialized()) { // Outer check extracted to helper
            initializeSafely();
        }
        return retrieveInstance(); // Access extracted to helper
    }

    private boolean isNotInitialized() {
        return instance == null;
    }

    private void initializeSafely() {
        synchronized (this) {
            if (instance == null) {
                instance = "lazy";
            }
        }
    }

    private String retrieveInstance() {
        return instance; // Access outside synchronized block
    }
}

PosCase2_Var5.java

This is still a read-outside-sync / initialize-inside-sync pattern. Restructuring it as an early return does not repair the memory-visibility problem.

// Double-checked locking on non-volatile immutable-type field with field access outside synchronized block should be flagged.
package scensct.var.pos;

public class PosCase2_Var5 {
    private String instance;

    public String getInstance() {
        // Restructured control flow with early return for null after sync
        if (instance != null) {
            return instance; // Early return, but still access outside sync
        }
        synchronized (this) {
            if (instance == null) {
                instance = "done";
            }
        }
        // Field accessed after synchronization block
        return instance;
    }
}

Cause analysis

The query appears too tied to one canonical AST shape for double-checked locking. As soon as the outer check is cached in a local, the inner initialization moves to a helper, or the method returns early on the fast path, the match falls apart.

That is a real modeling gap. The thread-safety problem depends on non-volatile publication and unsynchronized reads, not on whether the code is written in one exact syntactic form.

References

None known.