False Negative: CloseReader.ql misses unclosed streams once construction moves into helpers or alternate APIs.

[Carlson-JLQ]

Version
codeql 2.24.3

Checker

• Checker id: Likely Bugs/Resource Leaks/CloseReader.ql
• Checker description: This checker detects instances of Reader, InputStream, or ZipFile objects that are created but not guaranteed to be closed on method exit, potentially causing resource leaks.

Description of the false negative

All four samples still leave a reader or stream resource unclosed. The differences are superficial: one allocation is hidden behind a helper, one uses Files.newInputStream(...) instead of new FileInputStream(...), one leaks a custom InputStream, and one leaks a BufferedReader built around a parameter stream.

None of those variations change the resource-management obligation.

Affected test cases

PosCase1_Var4.java

The stream is created in openStream() and immediately discarded by the caller. That is still a leak.

PosCase1_Var5.java

Files.newInputStream(...) returns an InputStream that still needs to be closed.

PosCase2.java

The custom stream object is allocated, used, and never closed.

PosCase4.java

The BufferedReader wraps a parameter stream and is never closed, so the wrapper resource itself leaks.

Reproduction code

PosCase1_Var4.java

// FileInputStream created but not closed should be flagged as resource leak.
package scensct.var.pos;

import java.io.FileInputStream;
import java.io.IOException;

public class PosCase1_Var4 {
    // Variant 4: Extract creation to a helper method
    private FileInputStream openStream() throws IOException {
        return new FileInputStream("test.txt");
    }

    public void readFile() throws IOException {
        openStream(); // Returned stream is not assigned or closed
    }
}

PosCase1_Var5.java

// FileInputStream created but not closed should be flagged as resource leak.
package scensct.var.pos;

import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class PosCase1_Var5 {
    // Variant 5: Use Files.newInputStream (still an InputStream)
    public void readFile() throws IOException {
        Files.newInputStream(Paths.get("test.txt"));
        // Not closed
    }
}

PosCase2.java

// Custom InputStream without close() method created but not closed should be flagged.
package scensct.core.pos;

import java.io.InputStream;
import java.io.IOException;

public class PosCase2 {
    // Custom InputStream that does not declare close()
    static class CustomStream extends InputStream {
        @Override
        public int read() {
            return -1;
        }
        // No close() method overridden
    }

    public void useStream() {
        InputStream stream = new CustomStream(); // Instantiation with assignment
        try {
            stream.read(); // Use the stream to emphasize it's a resource
        } catch (IOException e) {
            // Ignore for test purposes
        }
        // Not closed
    }
}

PosCase4.java

// Wrapper BufferedReader around parameter InputStream not closed should be flagged.
package scensct.core.pos;

import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;

public class PosCase4 {
    public void wrapParameter(InputStream paramStream) {
        BufferedReader reader = new BufferedReader(new InputStreamReader(paramStream)); // Wrapper assigned but not closed
    }
}

Cause analysis

The misses point to incomplete source modeling in Likely Bugs/Resource Leaks/CloseReader.ql. The rule seems strongest on very direct allocation forms, but it loses coverage once the resource comes back from a helper or from another factory API.

That is a problem for a leak checker. Real code uses utility methods and alternative constructors all the time, and the need to close the returned resource does not disappear when the allocation becomes one step less direct.

[redsun82]

Hi @Carlson-JLQ, thanks for filing this. The CloseReader query intentionally focuses on common direct-allocation shapes, and we're aware it doesn't follow resources through helpers or factory APIs. We'll keep this on file, but it's unlikely to move up in priority unless we start seeing it affect real scan results.