Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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", "log4j.astub"})
public class RLCCalledMethodsChecker extends CalledMethodsChecker {

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

import java.io.PrintStream;
import java.io.PrintWriter;
import org.checkerframework.checker.lock.qual.GuardSatisfied;
import org.checkerframework.dataflow.qual.SideEffectFree;

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

public class Throwable {
@SideEffectFree
public void printStackTrace();

@SideEffectFree
public void printStackTrace(PrintStream s);

@SideEffectFree
public void printStackTrace(PrintWriter s);
}

package java.io;

import java.io.IOException;
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;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.apache.log4j;

import org.checkerframework.dataflow.qual.SideEffectFree;

class Category {
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.

https://logging.apache.org/log4j/1.x/apidocs/org/apache/log4j/Category.html says that this class has been deprecated since at least 2003. (!)

If you want to add log4j stubs, at least include the modern versions of the logging functions (i.e., in Logger) too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, that makes sense. I removed the old Category-only coverage and updated the PR to use Logger instead. It now covers both Log4j 1.x and Log4j 2 with separate regression tests.

@SideEffectFree
public void debug(Object message);

@SideEffectFree
public void warn(Object message);

@SideEffectFree
public void error(Object message);
}
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,30 @@
// 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;

final class TestAutoCloseable implements AutoCloseable {
@Override
public void close() {}
}

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

@Override
@EnsuresCalledMethods(
value = {"this.first", "this.second"},
methods = "close")
public void close() {
try {
first.close();
second.close();
} catch (Exception e) {
throw new AssertionError(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// 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 java.io.IOException;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.mustcall.qual.Owning;

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

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

@Override
@EnsuresCalledMethods(
value = {"this.first", "this.second"},
methods = "close")
// The exceptional postcondition is still expected to fail: if first.close() throws,
// then second.close() is not reached.
// :: error: contracts.exceptional.postcondition
public void close() {
try {
first.close();
second.close();
} catch (IOException e) {
throw new AssertionError(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// RLC-specific stubs mark these Log4j logging methods as SideEffectFree so that
// diagnostic logging after a resource is closed does not wipe out the close fact.
// This file checks the Object-taking debug/warn/error overloads that were added.

package org.apache.log4j;

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

class Category {
public void debug(Object message) {}

public void error(Object message) {}

public void warn(Object message) {}
}

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

class Log4jDebugObject implements Closeable {
private final Category logger = new Category();
private @Owning CloseableResource resource = new CloseableResource();

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

class Log4jWarnObject implements Closeable {
private final Category logger = new Category();
private @Owning CloseableResource resource = new CloseableResource();

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

class Log4jErrorObject implements Closeable {
private final Category logger = new Category();
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,52 @@
// 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;

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

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