Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* CalledMethodsChecker} used as a subchecker in the ResourceLeakChecker, and never independently.
* Runs the MustCallChecker as a subchecker in order to share the CFG.
*/
@StubFiles("IOUtils.astub")
@StubFiles({"IOUtils.astub"})
public class RLCCalledMethodsChecker extends CalledMethodsChecker {

/** Creates a RLCCalledMethodsChecker. */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package java.lang;

import org.checkerframework.checker.lock.qual.GuardSatisfied;
import org.checkerframework.dataflow.qual.SideEffectFree;

public interface AutoCloseable {
@SideEffectFree
void close(@GuardSatisfied AutoCloseable this) throws Exception;
}

package java.io;

import org.checkerframework.checker.lock.qual.GuardSatisfied;
import org.checkerframework.dataflow.qual.SideEffectFree;

public interface Closeable extends AutoCloseable {
@SideEffectFree
public void close(@GuardSatisfied Closeable this) throws IOException;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
iamsanjaymalakar marked this conversation as resolved.
7 changes: 0 additions & 7 deletions checker/tests/resourceleak/TwoResourcesECM.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,12 @@
class TwoResourcesECM {
@Owning Socket s1, s2;

// The contracts.postcondition error below is thrown because s1 is not final,
// and therefore might theoretically be side-effected by the call to s2.close()
// even on the non-exceptional path. See ReplicaInputStreams.java for a variant
// of this test where such an error is not issued. Because this method can leak
// along both regular and exceptional exits, both errors are issued.
//
// The contracts.exceptional.postcondition error is thrown because destructors
// have to close their resources even on exception. If s1.close() throws an
// exception, then s2.close() will not be called.
@EnsuresCalledMethods(
value = {"this.s1", "this.s2"},
methods = {"close"})
// :: error: [contracts.postcondition]
// :: error: [contracts.exceptional.postcondition]
public void dispose() throws IOException {
s1.close();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RLC uses Called Methods facts to remember that a resource has already been closed.
// This test checks that the RLC-specific SideEffectFree stub for AutoCloseable.close()
// preserves those facts across another close call in the same destructor, rather than
// conservatively forgetting them after the first invocation.

import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.mustcall.qual.Owning;

class AutoCloseableClose implements AutoCloseable {
private @Owning AutoCloseable first = new AutoCloseableResource();
private @Owning AutoCloseable second = new AutoCloseableResource();

@Override
@EnsuresCalledMethods(
value = {"this.first", "this.second"},
methods = "close")
public void close() {
try {
first.close();
second.close();
} catch (Exception e) {
throw new AssertionError(e);
}
}
Comment thread
iamsanjaymalakar marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RLC uses Called Methods facts to remember that a resource has already been closed.
// This test checks that the RLC-specific SideEffectFree stub for Closeable.close()
// preserves those facts across another close call in the same destructor, rather than
// conservatively forgetting them after the first invocation.

import java.io.Closeable;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.mustcall.qual.Owning;

class CloseableClose implements Closeable {
private @Owning Closeable first = new CloseableResource();
private @Owning Closeable second = new CloseableResource();

@Override
@EnsuresCalledMethods(
value = {"this.first", "this.second"},
methods = "close")
public void close() {
try {
try {
first.close();
} catch (Exception ignored) {
}
second.close();
} catch (Exception e) {
throw new AssertionError(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Class representing reources that implements Closeable and Autocloseable to be used by other tests
// in this directory.
Comment on lines +1 to +2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typos in the header comment.

"reources" → "resources"; "Autocloseable" → "AutoCloseable".

✏️ Proposed fix
-// Class representing reources that implements Closeable and Autocloseable to be used by other tests
-// in this directory.
+// Classes representing resources that implement Closeable and AutoCloseable, to be used by other
+// tests in this directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@checker/tests/resourceleak/sideeffect-free-stubs-tests/CloseableResource.java`
around lines 1 - 2, Fix the typos in the file header comment for the
CloseableResource class: change "reources" to "resources" and "Autocloseable" to
"AutoCloseable" so the comment correctly reads that the class represents
resources that implement Closeable and AutoCloseable for use by other tests.


import java.io.Closeable;

final class CloseableResource implements Closeable {
@Override
public void close() {}
}

final class AutoCloseableResource implements AutoCloseable {
@Override
public void close() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// This test covers the Log4j 1.x API in package org.apache.log4j.
// The RLC-specific stub marks logging methods as @SideEffectFree, so logging after a resource is
// closed should not wipe out the close fact.

import java.io.Closeable;
import org.apache.log4j.Logger;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.mustcall.qual.Owning;

class Log4j1DebugObject implements Closeable {
private final Logger logger = Logger.getLogger("Log4j1DebugObject");
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.debug("after close");
}
}

class Log4j1DebugWithThrowable implements Closeable {
private final Logger logger = Logger.getLogger("Log4j1DebugWithThrowable");
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.debug("after close", new RuntimeException());
}
}

class Log4j1InfoObject implements Closeable {
private final Logger logger = Logger.getLogger("Log4j1InfoObject");
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.info("after close");
}
}

class Log4j1WarnObject implements Closeable {
private final Logger logger = Logger.getLogger("Log4j1WarnObject");
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.warn("after close");
}
}

class Log4j1ErrorObject implements Closeable {
private final Logger logger = Logger.getLogger("Log4j1ErrorObject");
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.error("after close");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// This test covers the Log4j 2.x API in package org.apache.logging.log4j.
// real library API. The RLC-specific stub marks logging methods as @SideEffectFree, so logging
// after a resource is closed should not wipe out the close fact.
Comment on lines +1 to +3
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update stale/incorrect header comment.

Line 2 states "The RLC-specific stub marks logging methods as @SideEffectFree", but per the PR discussion the Log4j stubs were moved to framework/src/main/java/org/checkerframework/framework/stub/jdk.astub so they load for all checkers — they are no longer RLC-specific. Line 1 also reads like a broken sentence ("... package org.apache.logging.log4j. // real library API."). Please rewrite for accuracy and readability.

📝 Suggested rewrite
-// This test covers the Log4j 2.x API in package org.apache.logging.log4j.
-// real library API. The RLC-specific stub marks logging methods as `@SideEffectFree`, so logging
-// after a resource is closed should not wipe out the close fact.
+// This test covers the Log4j 2.x API in package org.apache.logging.log4j.
+// Stubs in framework/src/main/java/org/checkerframework/framework/stub/jdk.astub mark the
+// logging methods as `@SideEffectFree`, so logging after a resource is closed should not wipe
+// out the close fact.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// This test covers the Log4j 2.x API in package org.apache.logging.log4j.
// real library API. The RLC-specific stub marks logging methods as @SideEffectFree, so logging
// after a resource is closed should not wipe out the close fact.
// This test covers the Log4j 2.x API in package org.apache.logging.log4j.
// Stubs in framework/src/main/java/org/checkerframework/framework/stub/jdk.astub mark the
// logging methods as `@SideEffectFree`, so logging after a resource is closed should not wipe
// out the close fact.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@checker/tests/resourceleak/sideeffect-free-stubs-tests/Log4j2ObjectCalls.java`
around lines 1 - 3, Update the file header comment to correct the broken
sentence and reflect that the Log4j stubs are no longer RLC-specific: rewrite
the first two lines so they read clearly (e.g., "This test covers the Log4j 2.x
API in package org.apache.logging.log4j; it uses the real library API.") and
change the second sentence to state that the Log4j stubs now live under
framework/src/main/java/org/checkerframework/framework/stub/jdk.astub and
therefore apply to all checkers (remove any mention of "RLC-specific"). Ensure
the new comment is concise and accurate and replace the existing erroneous
lines.


import java.io.Closeable;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.mustcall.qual.Owning;

class Log4j2DebugObject implements Closeable {
private static final Logger logger = LogManager.getLogger();
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.debug("after close");
}
}

class Log4j2DebugVarargs implements Closeable {
private static final Logger logger = LogManager.getLogger();
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.debug("closed resource {}", "name");
}
}

class Log4j2InfoObject implements Closeable {
private static final Logger logger = LogManager.getLogger();
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.info("after close");
}
}

class Log4j2WarnObject implements Closeable {
private static final Logger logger = LogManager.getLogger();
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.warn("after close");
}
}

class Log4j2ErrorObject implements Closeable {
private static final Logger logger = LogManager.getLogger();
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.error("after close");
}
}

class Log4j2ErrorWithThrowable implements Closeable {
private static final Logger logger = LogManager.getLogger();
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
logger.error("after close", new RuntimeException());
}
}
Comment thread
iamsanjaymalakar marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// RLC-specific stubs mark Throwable.printStackTrace overloads as SideEffectFree so
// that debugging/logging after a resource is closed does not wipe out the close fact.
// This file checks the no-arg, PrintStream, and PrintWriter variants.

import java.io.Closeable;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.mustcall.qual.Owning;

class ThrowablePrintStackTrace implements Closeable {
private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
new RuntimeException("debug").printStackTrace();
}
}

class ThrowablePrintStackTracePrintStream implements Closeable {
private static final PrintStream STREAM = System.err;

private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
new RuntimeException("debug").printStackTrace(STREAM);
}
}

class ThrowablePrintStackTracePrintWriter implements Closeable {
private static final PrintWriter WRITER = new PrintWriter(new StringWriter());

private @Owning CloseableResource resource = new CloseableResource();

@Override
@EnsuresCalledMethods(value = "this.resource", methods = "close")
public void close() {
resource.close();
new RuntimeException("debug").printStackTrace(WRITER);
}
}
Loading
Loading