Skip to content

Commit

Permalink
Improve resource leakage detection testing
Browse files Browse the repository at this point in the history
This change makes various improvements to the support for
testing resource leakage detection.

Adds CloseGuard.Tracker to allow tests to track the lifecycle of
CloseGuard allocation sites directly.

Adds CloseGuardSupport to use CloseGuard.Tracker to provide:
* A BiConsumer that can be used to help check that objects which
own resources protected by CloseGuard correctly detect resource
leakage.
* A TestRule that allows tests to check that system code does
not leak resources, especially under error conditions.

Adds a new ResourceLeakageDetector that uses reflection to
access CloseGuardSupport. This can safely be used in code that
needs to compile and run on OpenJDK.

Adds TestCaseWithRules to allow the TestRule above to be used
with JUnit 3 style tests without converting the whole test to
JUnit 4 style.

Changed libcore.java.lang.ProcessBuilderTest to use TestCase as
ProcessBuilder does not have any CloseGuard protected resources.
That allows AbstractResourceLeakageDetectorTestCase to be
deleted.

Changed RandomAccessFileTest to use TestCaseWithRules and
ResourceLeakageDetector. Fixed issues that it highlighted,
fixed testRandomAccessFileHasCleanupFinalizer test and removed
it from expectations/knownfailures.txt.

Adds core-test-rules/-hostdex targets to encapsulate TestRule
and related classes that can be shared with other projects.

Removes the following now unused classes:
* CloseGuardMonitor
* AbstractResourceLeakageDetectorTest
* The old ResourceLeakageDetector and associated test.

Bug: 31542223
Test: Ran tests on both Vogar and CTS
Change-Id: I8f802b52fdbeac0a30f339a9ceca5d2eaaafd180
  • Loading branch information
paulduffin committed Oct 3, 2016
1 parent 24e3caf commit fe0ee8e
Show file tree
Hide file tree
Showing 12 changed files with 1,004 additions and 342 deletions.
31 changes: 27 additions & 4 deletions JavaLibrary.mk
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ core_resource_dirs := \
luni/src/main/java \
ojluni/src/main/resources/
test_resource_dirs := $(call all-core-resource-dirs,test)
test_src_files := $(call all-test-java-files-under,dalvik dom harmony-tests json luni xml)
test_src_files := $(call all-test-java-files-under,dalvik dalvik/test-rules dom harmony-tests json luni xml)
ojtest_src_files := $(call all-test-java-files-under,ojluni)

ifeq ($(EMMA_INSTRUMENT),true)
Expand Down Expand Up @@ -166,6 +166,24 @@ LOCAL_REQUIRED_MODULES := tzdata
LOCAL_CORE_LIBRARY := true
include $(BUILD_JAVA_LIBRARY)

# Build libcore test rules for target
include $(CLEAR_VARS)
LOCAL_SRC_FILES := $(call all-java-files-under, dalvik/test-rules/src/main test-rules/src/main)
LOCAL_NO_STANDARD_LIBRARIES := true
LOCAL_MODULE := core-test-rules
LOCAL_JAVA_LIBRARIES := core-all core-junit
LOCAL_STATIC_JAVA_LIBRARIES := junit4-target
include $(BUILD_STATIC_JAVA_LIBRARY)

# Build libcore test rules for host
include $(CLEAR_VARS)
LOCAL_SRC_FILES := $(call all-java-files-under, dalvik/test-rules/src/main test-rules/src/main)
LOCAL_NO_STANDARD_LIBRARIES := true
LOCAL_MODULE := core-test-rules-hostdex
LOCAL_JAVA_LIBRARIES := core-oj-hostdex core-libart-hostdex core-junit-hostdex
LOCAL_STATIC_JAVA_LIBRARIES := junit4-target-hostdex
include $(BUILD_HOST_DALVIK_JAVA_LIBRARY)

include $(CLEAR_VARS)
LOCAL_SRC_FILES := $(non_openjdk_java_files) $(android_icu4j_src_files)
LOCAL_JAVA_RESOURCE_DIRS := $(android_icu4j_resource_dirs)
Expand All @@ -189,7 +207,12 @@ LOCAL_SRC_FILES := $(test_src_files)
LOCAL_JAVA_RESOURCE_DIRS := $(test_resource_dirs)
LOCAL_NO_STANDARD_LIBRARIES := true
LOCAL_JAVA_LIBRARIES := core-oj core-libart okhttp core-junit junit4-target bouncycastle mockito-target
LOCAL_STATIC_JAVA_LIBRARIES := core-tests-support sqlite-jdbc mockwebserver nist-pkix-tests
LOCAL_STATIC_JAVA_LIBRARIES := \
core-test-rules \
core-tests-support \
mockwebserver \
nist-pkix-tests \
sqlite-jdbc
LOCAL_JAVACFLAGS := $(local_javac_flags)
LOCAL_JAVA_LANGUAGE_VERSION := 1.8
LOCAL_MODULE := core-tests
Expand Down Expand Up @@ -329,14 +352,14 @@ LOCAL_JAVA_LIBRARIES := core-all-hostdex
LOCAL_CORE_LIBRARY := true
include $(BUILD_HOST_DALVIK_JAVA_LIBRARY)

# Make the core-tests library.
# Make the core-tests-hostdex library.
ifeq ($(LIBCORE_SKIP_TESTS),)
include $(CLEAR_VARS)
LOCAL_SRC_FILES := $(test_src_files)
LOCAL_JAVA_RESOURCE_DIRS := $(test_resource_dirs)
LOCAL_NO_STANDARD_LIBRARIES := true
LOCAL_JAVA_LIBRARIES := core-oj-hostdex core-libart-hostdex okhttp-hostdex bouncycastle-hostdex core-junit-hostdex junit4-target-hostdex core-tests-support-hostdex mockito-api-hostdex
LOCAL_STATIC_JAVA_LIBRARIES := sqlite-jdbc-host mockwebserver-host nist-pkix-tests-host
LOCAL_STATIC_JAVA_LIBRARIES := sqlite-jdbc-host mockwebserver-host nist-pkix-tests-host core-test-rules-hostdex
LOCAL_JAVACFLAGS := $(local_javac_flags)
LOCAL_MODULE_TAGS := optional
LOCAL_JAVA_LANGUAGE_VERSION := 1.8
Expand Down
74 changes: 72 additions & 2 deletions dalvik/src/main/java/dalvik/system/CloseGuard.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ public final class CloseGuard {
*/
private static volatile Reporter REPORTER = new DefaultReporter();

/**
* The default {@link Tracker}.
*/
private static final DefaultTracker DEFAULT_TRACKER = new DefaultTracker();

/**
* Hook for customizing how CloseGuard issues are tracked.
*/
private static volatile Tracker currentTracker = DEFAULT_TRACKER;

/**
* Returns a CloseGuard instance. If CloseGuard is enabled, {@code
* #open(String)} can be used to set up the instance to warn on
Expand All @@ -138,6 +148,13 @@ public static void setEnabled(boolean enabled) {
ENABLED = enabled;
}

/**
* True if CloseGuard mechanism is enabled.
*/
public static boolean isEnabled() {
return ENABLED;
}

/**
* Used to replace default Reporter used to warn of CloseGuard
* violations. Must be non-null.
Expand All @@ -156,6 +173,32 @@ public static Reporter getReporter() {
return REPORTER;
}

/**
* Sets the {@link Tracker} that is notified when resources are allocated and released.
*
* <p>This is only intended for use by {@code dalvik.system.CloseGuardSupport} class and so
* MUST NOT be used for any other purposes.
*
* @throws NullPointerException if tracker is null
*/
public static void setTracker(Tracker tracker) {
if (tracker == null) {
throw new NullPointerException("tracker == null");
}
currentTracker = tracker;
}

/**
* Returns {@link #setTracker(Tracker) last Tracker that was set}, or otherwise a default
* Tracker that does nothing.
*
* <p>This is only intended for use by {@code dalvik.system.CloseGuardSupport} class and so
* MUST NOT be used for any other purposes.
*/
public static Tracker getTracker() {
return currentTracker;
}

private CloseGuard() {}

/**
Expand All @@ -178,6 +221,7 @@ public void open(String closer) {
}
String message = "Explicit termination method '" + closer + "' not called";
allocationSite = new Throwable(message);
currentTracker.open(allocationSite);
}

private Throwable allocationSite;
Expand All @@ -187,6 +231,7 @@ public void open(String closer) {
* finalization.
*/
public void close() {
currentTracker.close(allocationSite);
allocationSite = null;
}

Expand All @@ -208,11 +253,36 @@ public void warnIfOpen() {
REPORTER.report(message, allocationSite);
}

/**
* Interface to allow customization of tracking behaviour.
*
* <p>This is only intended for use by {@code dalvik.system.CloseGuardSupport} class and so
* MUST NOT be used for any other purposes.
*/
public interface Tracker {
void open(Throwable allocationSite);
void close(Throwable allocationSite);
}

/**
* Default tracker which does nothing special and simply leaves it up to the GC to detect a
* leak.
*/
private static final class DefaultTracker implements Tracker {
@Override
public void open(Throwable allocationSite) {
}

@Override
public void close(Throwable allocationSite) {
}
}

/**
* Interface to allow customization of reporting behavior.
*/
public static interface Reporter {
public void report (String message, Throwable allocationSite);
public interface Reporter {
void report (String message, Throwable allocationSite);
}

/**
Expand Down
119 changes: 0 additions & 119 deletions dalvik/src/test/java/dalvik/system/CloseGuardMonitor.java

This file was deleted.

Loading

0 comments on commit fe0ee8e

Please sign in to comment.