False Negative: Delegating and wrapped forms of unsynchronized overrides are not recognized as real override risks.

[Carlson-JLQ]

False Negative: Delegating and wrapped forms of unsynchronized overrides are not recognized as real override risks.

Version
2.48.0

Checker

• Checker id: UnsynchronizedOverridesSynchronized
• Checker description: This checker detects unsynchronized methods that override a synchronized method in a superclass, unless the override is in java.io.InputStream or the method body consists solely of a super call to the overridden method or a constant return.

Description of the false negative

diff/output marks 3 false negatives for UnsynchronizedOverridesSynchronized. Across these samples, the benchmark is still exercising the same underlying bug pattern, but the implementation appears to lose track of it once the source deviates from one canonical shape. The recurring themes are synchronization patterns, suppression or annotations.

Affected test cases

PosCase1.java

Source: output/codeql/NonSynchronizedOverride/src/main/java/scensct/core/pos/PosCase1.java

This is a concurrency-pattern sample where the same bug or non-bug is expressed through synchronization primitives rather than a single canonical syntax form. The concurrency bug is still present, but it is hidden behind a slightly different arrangement of synchronization or field access. That is also how the benchmark classifies the sample: A non-synchronized method overrides a synchronized method and contains no method calls in its body should be flagged as positive.

// A non-synchronized method overrides a synchronized method and contains no method calls in its body should be flagged as positive.
package scensct.core.pos;

public class PosCase1 {
    static class Parent {
        synchronized void method() {
            // synchronized method
        }
    }
    
    static class Child extends Parent {
        @Override
        void method() { // [REPORTED LINE]
            // No method calls in body - should be flagged
        }
    }
}

PosCase3.java

Source: output/codeql/NonSynchronizedOverride/src/main/java/scensct/core/pos/PosCase3.java

This is a concurrency-pattern sample where the same bug or non-bug is expressed through synchronization primitives rather than a single canonical syntax form. The concurrency bug is still present, but it is hidden behind a slightly different arrangement of synchronization or field access. That is also how the benchmark classifies the sample: A non-synchronized method overrides a synchronized method and contains only super calls to the overridden method with non-variable arguments, and no method calls inside casting expressions should be flagged as positive.

// A non-synchronized method overrides a synchronized method and contains only super calls to the overridden method with non-variable arguments, and no method calls inside casting expressions should be flagged as positive.
package scensct.core.pos;

public class PosCase3 {
    static class Parent {
        synchronized void method(int x) {
            // synchronized method
        }
    }
    
    static class Child extends Parent {
        @Override
        void method(int x) { // [REPORTED LINE]
            // Contains only super call to overridden method with literal argument - should be flagged
            super.method(42);
        }
    }
}

PosCase1.java

Source: output/sonarqube/SynchronizedOverrideCheck/src/main/java/scensct/core/pos/PosCase1.java

This is a concurrency-pattern sample where the same bug or non-bug is expressed through synchronization primitives rather than a single canonical syntax form. The concurrency bug is still present, but it is hidden behind a slightly different arrangement of synchronization or field access. That is also how the benchmark classifies the sample: A non-synchronized method overrides a synchronized method from its parent class should be flagged as a violation.

// A non-synchronized method overrides a synchronized method from its parent class should be flagged as a violation.
package scensct.core.pos;

public class PosCase1 {
    // Parent class with synchronized method
    static class Parent {
        synchronized void synchronizedMethod() {
            System.out.println("Parent synchronized method");
        }
    }

    // Child class overriding without synchronized keyword
    static class Child extends Parent {
        @Override
        void synchronizedMethod() {  // Non-synchronized override of synchronized method // [REPORTED LINE]
            System.out.println("Child non-synchronized override");
        }
    }

    public static void main(String[] args) {
        new Child().synchronizedMethod();
    }
}

Cause analysis

The rule is documented as follows: This checker detects unsynchronized methods that override a synchronized method in a superclass, unless the override is in java.io.InputStream or the method body consists solely of a super call to the overridden method or a constant return.

In this set the problem shows up around synchronization patterns, suppression or annotations. The missed cases suggest that the implementation is narrower than the rule description and too dependent on one preferred AST shape.

Once the same behavior is expressed through a helper, an overload, a modifier such as abstract or native, a different comment position, or a slightly rearranged control-flow structure, the report disappears even though the benchmark still treats the case as in-scope.

For UnsynchronizedOverridesSynchronized, the practical improvement would be to generalize the match so it follows the underlying behavior rather than one exact spelling of it.

References

None known.

[eamonnmcmanus]

Hi, you logged 18 issues on this project in the space of 25 minutes. Those issues are hard to understand. It may be that there are some genuine improvements to be made here, but we feel that it is up to you and not the project maintainers to study these reports in detail and determine whether they are genuinely useful. The most useful reports are of issues that have arisen in real codebases: either problems that a check could have caught but did not, or false positives that occur often enough to be annoying.