Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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,32 @@
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 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,89 @@
import org.checkerframework.dataflow.qual.SideEffectFree;

// Log4j 1.x API stubs (package org.apache.log4j).
package org.apache.log4j;

class Logger {
@SideEffectFree public void trace(Object message);
@SideEffectFree public void trace(Object message, Throwable t);
@SideEffectFree public void debug(Object message);
@SideEffectFree public void debug(Object message, Throwable t);
@SideEffectFree public void info(Object message);
@SideEffectFree public void info(Object message, Throwable t);
@SideEffectFree public void warn(Object message);
@SideEffectFree public void warn(Object message, Throwable t);
@SideEffectFree public void error(Object message);
@SideEffectFree public void error(Object message, Throwable t);
@SideEffectFree public void fatal(Object message);
@SideEffectFree public void fatal(Object message, Throwable t);
@SideEffectFree public void log(Priority priority, Object message);
@SideEffectFree public void log(Priority priority, Object message, Throwable t);
@SideEffectFree public void log(String callerFQCN, Priority level, Object message, Throwable t);
}

// Log4j 2.x API stubs (package org.apache.logging.log4j).
package org.apache.logging.log4j;

import org.apache.logging.log4j.message.Message;

interface Logger {
@SideEffectFree void trace(CharSequence message);
@SideEffectFree void trace(CharSequence message, Throwable throwable);
@SideEffectFree void trace(Object message);
@SideEffectFree void trace(Object message, Throwable throwable);
@SideEffectFree void trace(String message);
@SideEffectFree void trace(String message, Throwable throwable);
@SideEffectFree void trace(String message, Object... params);
@SideEffectFree void trace(Message message);
@SideEffectFree void trace(Message message, Throwable throwable);

@SideEffectFree void debug(CharSequence message);
@SideEffectFree void debug(CharSequence message, Throwable throwable);
@SideEffectFree void debug(Object message);
@SideEffectFree void debug(Object message, Throwable throwable);
@SideEffectFree void debug(String message);
@SideEffectFree void debug(String message, Throwable throwable);
@SideEffectFree void debug(String message, Object... params);
@SideEffectFree void debug(Message message);
@SideEffectFree void debug(Message message, Throwable throwable);

@SideEffectFree void info(CharSequence message);
@SideEffectFree void info(CharSequence message, Throwable throwable);
@SideEffectFree void info(Object message);
@SideEffectFree void info(Object message, Throwable throwable);
@SideEffectFree void info(String message);
@SideEffectFree void info(String message, Throwable throwable);
@SideEffectFree void info(String message, Object... params);
@SideEffectFree void info(Message message);
@SideEffectFree void info(Message message, Throwable throwable);

@SideEffectFree void warn(CharSequence message);
@SideEffectFree void warn(CharSequence message, Throwable throwable);
@SideEffectFree void warn(Object message);
@SideEffectFree void warn(Object message, Throwable throwable);
@SideEffectFree void warn(String message);
@SideEffectFree void warn(String message, Throwable throwable);
@SideEffectFree void warn(String message, Object... params);
@SideEffectFree void warn(Message message);
@SideEffectFree void warn(Message message, Throwable throwable);

@SideEffectFree void error(CharSequence message);
@SideEffectFree void error(CharSequence message, Throwable throwable);
@SideEffectFree void error(Object message);
@SideEffectFree void error(Object message, Throwable throwable);
@SideEffectFree void error(String message);
@SideEffectFree void error(String message, Throwable throwable);
@SideEffectFree void error(String message, Object... params);
@SideEffectFree void error(Message message);
@SideEffectFree void error(Message message, Throwable throwable);

@SideEffectFree void fatal(CharSequence message);
@SideEffectFree void fatal(CharSequence message, Throwable throwable);
@SideEffectFree void fatal(Object message);
@SideEffectFree void fatal(Object message, Throwable throwable);
@SideEffectFree void fatal(String message);
@SideEffectFree void fatal(String message, Throwable throwable);
@SideEffectFree void fatal(String message, Object... params);
@SideEffectFree void fatal(Message message);
@SideEffectFree void fatal(Message message, Throwable throwable);
}
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,34 @@
// 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;

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")
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,87 @@
// This test covers the Log4j 1.x API in package org.apache.log4j.
// The local Logger class below is just a tiny stand-in for the 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.

package org.apache.log4j;
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.

The structure of this test is strange, given its stated purpose. Why use a mocked Logger object that's actually in org.apache.log4j rather than importing Logger in some other class and testing a use of it? My understanding of the goal of this PR is to make sure that logging code doesn't invalidate RLC facts in clients of the logging code, not in the logging code itself.

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.

Yeah, this makes sense. I've updated the test cases.


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

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

public void debug(Object message, Throwable t) {}

public void info(Object message) {}

public void warn(Object message) {}

public void error(Object message) {}
}

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

class Log4j1DebugObject implements Closeable {
private final Logger logger = new Logger();
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 = new Logger();
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 = new Logger();
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 = new Logger();
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 = new Logger();
private @Owning CloseableResource resource = new CloseableResource();

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