From 2aaa211739848c8fb188dc5fc5d8be27417a49ca Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 16 Dec 2017 09:33:40 +0100 Subject: [PATCH 01/20] Update to JUnit 4.12 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2753fc9..0c84f91 100644 --- a/pom.xml +++ b/pom.xml @@ -146,7 +146,7 @@ junit junit - 4.9 + 4.12 test From 44c6271304cef032fa4ed50e1907873232b89282 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 16 Dec 2017 12:49:30 +0100 Subject: [PATCH 02/20] Adjust a typo and unused imports --- src/main/java/org/kohsuke/file_leak_detector/AgentMain.java | 2 +- .../kohsuke/file_leak_detector/instrumented/SocketDemo.java | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java index 70871c9..f5ed351 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java +++ b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java @@ -249,7 +249,7 @@ static List createSpec() { java.net.Socket/ServerSocket uses SocketImpl, and this is where FileDescriptors are actually managed. - SocketInputStream/SocketOutputStream does not maintain a separate FileDescritor. + SocketInputStream/SocketOutputStream does not maintain a separate FileDescriptor. They just all piggy back on the same SocketImpl instance. */ new ClassTransformSpec("java/net/PlainSocketImpl", diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/SocketDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/SocketDemo.java index 1946470..4237403 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/SocketDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/SocketDemo.java @@ -1,11 +1,8 @@ package org.kohsuke.file_leak_detector.instrumented; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; From 565b5f3917d2ca000b4494db41cd0383bdd09d3d Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 29 Mar 2018 15:38:45 +0200 Subject: [PATCH 03/20] Set compiler to Java 1.7 --- pom.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pom.xml b/pom.xml index 0c84f91..d1e5be8 100644 --- a/pom.xml +++ b/pom.xml @@ -116,6 +116,14 @@ + + org.apache.maven.plugins + maven-compiler-plugin + + 1.7 + 1.7 + + From cda43d1e4724b901c32b7a438970bba5e54abd54 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Tue, 30 Oct 2018 07:23:40 +0100 Subject: [PATCH 04/20] Ignore IntelliJ project file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a20e547..2f12475 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ /.idea/*/* /.idea/* /.idea +/file-leak-detector.iml From 20a11ed1b2baaae671beea16d99dc3227b091776 Mon Sep 17 00:00:00 2001 From: M Ramon Leon Date: Tue, 30 Oct 2018 12:59:02 +0100 Subject: [PATCH 05/20] [JENKINS-53394] Check null options --- src/main/java/org/kohsuke/file_leak_detector/Main.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/kohsuke/file_leak_detector/Main.java b/src/main/java/org/kohsuke/file_leak_detector/Main.java index de175db..c52af5a 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/Main.java +++ b/src/main/java/org/kohsuke/file_leak_detector/Main.java @@ -70,7 +70,7 @@ public void run() throws Exception { System.out.println("Activating file leak detector at "+agentJar); // load a specified agent onto the JVM // pass the hidden option to prevent this from killing the target JVM if the options were wrong - api.getMethod("loadAgent",String.class,String.class).invoke(vm, agentJar.getPath(), "noexit,"+options); + api.getMethod("loadAgent",String.class,String.class).invoke(vm, agentJar.getPath(), options == null ? "noexit" : "noexit,"+options); } finally { api.getMethod("detach").invoke(vm); } From 15493b7826b4c2769305b43999e52e5a6f4956ab Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 30 Oct 2018 10:35:23 -0700 Subject: [PATCH 06/20] [maven-release-plugin] prepare release file-leak-detector-1.13 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index d1e5be8..b1e8645 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ org.kohsuke file-leak-detector - 1.13-SNAPSHOT + 1.13 File Leak Detector http://${project.artifactId}.kohsuke.org/ @@ -16,7 +16,7 @@ scm:git:git@github.com/kohsuke/${project.artifactId}.git scm:git:ssh://git@github.com/kohsuke/${project.artifactId}.git http://${project.artifactId}.kohsuke.org/ - HEAD + file-leak-detector-1.13 From ccb7f69ddb3def52622e0d4ccb2e5ecd5d9fba3e Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 30 Oct 2018 10:35:34 -0700 Subject: [PATCH 07/20] [maven-release-plugin] prepare for next development iteration --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index b1e8645..54a4973 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ org.kohsuke file-leak-detector - 1.13 + 1.14-SNAPSHOT File Leak Detector http://${project.artifactId}.kohsuke.org/ @@ -16,7 +16,7 @@ scm:git:git@github.com/kohsuke/${project.artifactId}.git scm:git:ssh://git@github.com/kohsuke/${project.artifactId}.git http://${project.artifactId}.kohsuke.org/ - file-leak-detector-1.13 + HEAD From 88e54233840d89c2b9216a37e0fd43869127e845 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 07:35:16 +0100 Subject: [PATCH 08/20] Enhance test, taken from #47 --- .../file_leak_detector/TransformerTest.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java b/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java index ecb631f..1c5f34b 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java +++ b/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java @@ -10,10 +10,13 @@ import org.kohsuke.file_leak_detector.transform.ClassTransformSpec; import org.kohsuke.file_leak_detector.transform.TransformerImpl; +import java.io.ByteArrayOutputStream; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; -import java.util.zip.ZipFile; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertTrue; /** * @author Kohsuke Kawaguchi @@ -23,7 +26,7 @@ public class TransformerTest { List specs = AgentMain.createSpec(); Class c; - + public TransformerTest(Class c) { this.c = c; } @@ -42,9 +45,15 @@ public void testInstrumentations() throws Exception { // o.write(data2); // o.close(); - CheckClassAdapter.verify(new ClassReader(data2), false, new PrintWriter(System.err)); + String errors; + ClassReader classReader = new ClassReader(data2); + try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + CheckClassAdapter.verify(classReader, false, new PrintWriter(baos)); + errors = new String(baos.toByteArray(), UTF_8); + } + assertTrue("Verification failed for " + c + "\n" + errors, errors.isEmpty()); } - + @Parameters public static List specs() throws Exception { List r = new ArrayList(); From 685e74b668e5df581fa4be8efc47641370482baf Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 08:26:10 +0100 Subject: [PATCH 09/20] Improve handling of Parameterized in unit-test and some other test improvements --- .../file_leak_detector/TransformerTest.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java b/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java index 1c5f34b..5a87563 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java +++ b/src/test/java/org/kohsuke/file_leak_detector/TransformerTest.java @@ -11,11 +11,13 @@ import org.kohsuke.file_leak_detector.transform.TransformerImpl; import java.io.ByteArrayOutputStream; +import java.io.InputStream; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; /** @@ -23,9 +25,9 @@ */ @RunWith(Parameterized.class) public class TransformerTest { - List specs = AgentMain.createSpec(); + private final static List specs = AgentMain.createSpec(); - Class c; + private final Class c; public TransformerTest(Class c) { this.c = c; @@ -36,7 +38,9 @@ public void testInstrumentations() throws Exception { TransformerImpl t = new TransformerImpl(specs); String name = c.getName().replace('.', '/'); - byte[] data = IOUtils.toByteArray(getClass().getClassLoader().getResourceAsStream(name + ".class")); + InputStream resource = getClass().getClassLoader().getResourceAsStream(name + ".class"); + assertNotNull("Could not load " + name + ".class", resource); + byte[] data = IOUtils.toByteArray(resource); byte[] data2 = t.transform(name,data); // File classFile = new File("/tmp/" + name + ".class"); @@ -45,7 +49,7 @@ public void testInstrumentations() throws Exception { // o.write(data2); // o.close(); - String errors; + final String errors; ClassReader classReader = new ClassReader(data2); try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { CheckClassAdapter.verify(classReader, false, new PrintWriter(baos)); @@ -54,10 +58,10 @@ public void testInstrumentations() throws Exception { assertTrue("Verification failed for " + c + "\n" + errors, errors.isEmpty()); } - @Parameters + @Parameters(name = "{index} - {0}") public static List specs() throws Exception { List r = new ArrayList(); - for (ClassTransformSpec s : AgentMain.createSpec()) { + for (ClassTransformSpec s : specs) { Class c = TransformerTest.class.getClassLoader().loadClass(s.name.replace('/', '.')); r.add(new Object[]{c}); } From d2c59de77bb3ec154abf96b9f4170aa5eaf5bc84 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 09:26:58 +0100 Subject: [PATCH 10/20] Fix IDE warning about generics --- .../file_leak_detector/transform/ClassTransformSpec.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/kohsuke/file_leak_detector/transform/ClassTransformSpec.java b/src/main/java/org/kohsuke/file_leak_detector/transform/ClassTransformSpec.java index 550af14..3d266c8 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/transform/ClassTransformSpec.java +++ b/src/main/java/org/kohsuke/file_leak_detector/transform/ClassTransformSpec.java @@ -12,7 +12,7 @@ public final class ClassTransformSpec { public final String name; /*package*/ Map methodSpecs = new HashMap(); - public ClassTransformSpec(Class clazz, MethodTransformSpec... methodSpecs) { + public ClassTransformSpec(Class clazz, MethodTransformSpec... methodSpecs) { this(clazz.getName().replace('.', '/'),methodSpecs); } From f81b5bb2e98a51adb0246d827f04f11088866a3b Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 09:28:03 +0100 Subject: [PATCH 11/20] Add a test for AgentMain which verifies that all classes from the spec are actually instrumented This uses new dependency on Mockito for testing with mock objects. --- pom.xml | 12 +++- .../file_leak_detector/AgentMainTest.java | 68 +++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java diff --git a/pom.xml b/pom.xml index 54a4973..8f389f5 100644 --- a/pom.xml +++ b/pom.xml @@ -53,6 +53,7 @@ **/TransformerTest.java + **/AgentMainTest.java -javaagent:"${project.build.directory}/file-leak-detector-${project.version}-jar-with-dependencies.jar" @@ -67,9 +68,8 @@ - - **/TransformerTest.java - + **/TransformerTest.java + **/AgentMainTest.java **/instrumented/*.java @@ -157,6 +157,12 @@ 4.12 test + + org.mockito + mockito-core + 3.2.4 + test + diff --git a/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java b/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java new file mode 100644 index 0000000..ee6b979 --- /dev/null +++ b/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java @@ -0,0 +1,68 @@ +package org.kohsuke.file_leak_detector; + +import org.junit.Test; +import org.kohsuke.file_leak_detector.transform.ClassTransformSpec; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.lang.instrument.ClassFileTransformer; +import java.lang.instrument.Instrumentation; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +public class AgentMainTest { + @Test + public void noDuplicateSpecs() { + final List specs = AgentMain.createSpec(); + + Set seenClasses = new HashSet<>(); + for (ClassTransformSpec spec : specs) { + assertTrue("Did have duplicate spec for class " + spec.name, seenClasses.add(spec.name)); + } + } + + @Test + public void testPreMain() throws Exception { + final List specs = AgentMain.createSpec(); + + final Set seenClasses = new HashSet<>(); + for (ClassTransformSpec spec : specs) { + seenClasses.add(spec.name); + } + + Instrumentation instrumentation = mock(Instrumentation.class); + Mockito.doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + for (Object obj : invocationOnMock.getArguments()) { + Class clazz = (Class) obj; + String name = clazz.getName().replace(".", "/"); + assertTrue("Tried to transform a class wihch is not contained in the specs: " + name + " (" + clazz + "), having remaining classes: " + seenClasses, + seenClasses.remove(name)); + } + return null; + } + }).when(instrumentation).retransformClasses((Class) any()); + + AgentMain.premain(null, instrumentation); + + verify(instrumentation, times(1)).addTransformer((ClassFileTransformer) any(), anyBoolean()); + verify(instrumentation, times(1)).retransformClasses((Class) any()); + + // the following two are not available in all JVMs + seenClasses.remove("sun/nio/ch/SocketChannelImpl"); + seenClasses.remove("java/net/AbstractPlainSocketImpl"); + + assertTrue("Had classes in the spec which were not instrumented: " + seenClasses, + seenClasses.isEmpty()); + } +} From 5d4bc3926c741de444006f25f52f9d70ae30ccd3 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 09:43:08 +0100 Subject: [PATCH 12/20] Add support for Files.newByteChannel() --- .../kohsuke/file_leak_detector/AgentMain.java | 17 +++++++++---- .../kohsuke/file_leak_detector/Listener.java | 23 +++++++++++------- .../instrumented/FileDemo.java | 24 ++++++++++++++++--- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java index f5ed351..aa31dca 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java +++ b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java @@ -21,9 +21,11 @@ import java.net.Socket; import java.net.SocketImpl; import java.nio.channels.FileChannel; +import java.nio.channels.SeekableByteChannel; import java.nio.channels.spi.AbstractInterruptibleChannel; import java.nio.channels.spi.AbstractSelectableChannel; import java.nio.channels.spi.AbstractSelector; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; import java.util.List; @@ -52,10 +54,10 @@ public class AgentMain { public static void agentmain(String agentArguments, Instrumentation instrumentation) throws Exception { premain(agentArguments,instrumentation); } - + public static void premain(String agentArguments, Instrumentation instrumentation) throws Exception { int serverPort = -1; - + if(agentArguments!=null) { // used by Main to prevent the termination of target JVM boolean quit = true; @@ -227,6 +229,12 @@ static List createSpec() { new ClassTransformSpec(FileChannel.class, new ReturnFromStaticMethodInterceptor("open", "(Ljava/nio/file/Path;Ljava/util/Set;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/channels/FileChannel;", 4, "open_filechannel", FileChannel.class, Path.class)), + /* + SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) + */ + new ClassTransformSpec(Files.class, + new ReturnFromStaticMethodInterceptor("newByteChannel", + "(Ljava/nio/file/Path;Ljava/util/Set;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/channels/SeekableByteChannel;", 4, "open_filechannel", SeekableByteChannel.class, Path.class)), /* * Detect new Pipes */ @@ -353,6 +361,7 @@ protected void append(CodeGenerator g) { /** * Used to intercept {@link java.net.PlainSocketImpl#accept(SocketImpl)} */ + @SuppressWarnings("JavadocReference") private static class AcceptInterceptor extends MethodAppender { public AcceptInterceptor(String name, String desc) { super(name,desc); @@ -398,7 +407,7 @@ private OpenInterceptionAdapter(MethodVisitor base, int access, String desc) { * Decide if this is the method that needs interception. */ protected abstract boolean toIntercept(String owner, String name); - + protected Class getExpectedException() { return IOException.class; } @@ -407,7 +416,7 @@ protected Class getExpectedException() { public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { if(toIntercept(owner,name)) { Type exceptionType = Type.getType(getExpectedException()); - + CodeGenerator g = new CodeGenerator(mv); Label s = new Label(); // start of the try block Label e = new Label(); // end of the try block diff --git a/src/main/java/org/kohsuke/file_leak_detector/Listener.java b/src/main/java/org/kohsuke/file_leak_detector/Listener.java index 55f086b..715d0fa 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/Listener.java +++ b/src/main/java/org/kohsuke/file_leak_detector/Listener.java @@ -15,6 +15,7 @@ import java.net.SocketImpl; import java.nio.channels.FileChannel; import java.nio.channels.Pipe; +import java.nio.channels.SeekableByteChannel; import java.nio.channels.Selector; import java.nio.channels.SocketChannel; import java.nio.file.Path; @@ -28,7 +29,7 @@ /** * Intercepted JDK calls land here. - * + * * @author Kohsuke Kawaguchi */ public class Listener { @@ -60,7 +61,7 @@ public void dump(String prefix, PrintWriter pw) { pw.println("\tat " + trace[i]); pw.flush(); } - + public boolean exclude() { if(EXCLUDES.isEmpty()) { return false; @@ -75,7 +76,7 @@ public boolean exclude() { break; } } - + // check the rest for (; i < trace.length; i++) { String t = trace[i].toString(); @@ -244,7 +245,7 @@ public void dump(String prefix, PrintWriter ps) { public static PrintWriter ERROR = new PrintWriter(System.err); /** - * Allows to provide stacktrace-lines which cause the element to be excluded + * Allows to provide stacktrace-lines which cause the element to be excluded */ public static final List EXCLUDES = new ArrayList(); @@ -270,7 +271,7 @@ public void dump(String prefix, PrintWriter ps) { public static boolean isAgentInstalled() { return AGENT_INSTALLED; } - + public static synchronized void makeStrong() { TABLE = new LinkedHashMap(TABLE); } @@ -309,6 +310,10 @@ public static synchronized void open_filechannel(FileChannel fileChannel, Path p open(fileChannel, path.toFile()); } + public static synchronized void open_filechannel(SeekableByteChannel byteChannel, Path path) { + open(byteChannel, path.toFile()); + } + public static synchronized void openSelector(Object _this) { if (_this instanceof Selector) { put(_this, new SelectorRecord((Selector)_this)); @@ -353,11 +358,11 @@ public static synchronized void openSocket(Object _this) { } } } - + public static synchronized List getCurrentOpenFiles() { return new ArrayList(TABLE.values()); } - + private static synchronized void put(Object _this, Record r) { // handle excludes if(r.exclude()) { @@ -435,7 +440,7 @@ public static synchronized void outOfDescriptors() { tracing = false; } } - + private static String format(long time) { try { return new Date(time).toString(); @@ -445,7 +450,7 @@ private static String format(long time) { } private static Field SOCKETIMPL_SOCKET,SOCKETIMPL_SERVER_SOCKET; - + static { try { SOCKETIMPL_SOCKET = SocketImpl.class.getDeclaredField("socket"); diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java index 5cd02d6..e708c8e 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java @@ -9,6 +9,8 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.nio.channels.FileChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.Files; import java.nio.file.StandardOpenOption; import org.junit.After; @@ -20,7 +22,7 @@ import org.kohsuke.file_leak_detector.Listener.Record; /** - * + * * @author Andreas Dangel */ public class FileDemo { @@ -29,7 +31,8 @@ public class FileDemo { @BeforeClass public static void setup() { - assertTrue(Listener.isAgentInstalled()); + assertTrue("This test expects the Java Agent to be installed via commandline options", + Listener.isAgentInstalled()); Listener.TRACE = new PrintWriter(output); } @@ -41,7 +44,7 @@ public void prepareOutput() throws Exception { @After public void cleanup () { - tempFile.delete(); + assertTrue(!tempFile.exists() || tempFile.delete()); } @Test @@ -70,6 +73,21 @@ public void openCloseFileChannel() throws Exception { assertTrue(traceOutput.contains("Closed " + tempFile)); } + @Test + public void openCloseFilesNewByteChannel() throws Exception { + // this triggers the following method + // FileDescriptor sun.nio.fs.UnixChannelFactory.open(...) + SeekableByteChannel fileChannel = Files.newByteChannel(tempFile.toPath(), StandardOpenOption.APPEND); + assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + + fileChannel.close(); + assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempFile)); + assertTrue(traceOutput.contains("Closed " + tempFile)); + } + private static FileRecord findFileRecord(File file) { for (Record record : Listener.getCurrentOpenFiles()) { if (record instanceof FileRecord) { From b1ed145594f1f63244bdcb5ac80eba255f00fee6 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 09:43:45 +0100 Subject: [PATCH 13/20] Try to instrument some classes which are not available in all versions of the JVM --- .../kohsuke/file_leak_detector/AgentMain.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java index aa31dca..97b7a35 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java +++ b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java @@ -135,8 +135,8 @@ public void run() { Listener.AGENT_INSTALLED = true; instrumentation.addTransformer(new TransformerImpl(createSpec()),true); - - instrumentation.retransformClasses( + + List classes = Arrays.asList(new Class[] { FileInputStream.class, FileOutputStream.class, RandomAccessFile.class, @@ -145,8 +145,13 @@ public void run() { AbstractSelectableChannel.class, AbstractInterruptibleChannel.class, FileChannel.class, - AbstractSelector.class - ); + AbstractSelector.class, + Files.class}); + + addIfFound(classes, "sun/nio/ch/SocketChannelImpl"); + addIfFound(classes, "java/net/AbstractPlainSocketImpl"); + + instrumentation.retransformClasses(classes.toArray(new Class[0])); // Socket.class, @@ -158,6 +163,14 @@ public void run() { runHttpServer(serverPort); } + private static void addIfFound(List classes, String className) { + try { + classes.add(Class.forName(className)); + } catch(ClassNotFoundException e) { + // ignored here + } + } + private static void runHttpServer(int port) throws IOException { final ServerSocket ss = new ServerSocket(); ss.bind(new InetSocketAddress("localhost", port)); From a52b9e42e5b646c3ef597aa5a1877cd73afd53e3 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 09:54:04 +0100 Subject: [PATCH 14/20] Add a commented test which only works with Java 8+ --- .../file_leak_detector/instrumented/FileDemo.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java index e708c8e..3212257 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java @@ -88,6 +88,20 @@ public void openCloseFilesNewByteChannel() throws Exception { assertTrue(traceOutput.contains("Closed " + tempFile)); } + /* This is only available in Java 8 and newer... + @Test + public void openCloseFileLines() throws Exception { + Stream stream = Files.lines(tempFile.toPath()); + assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + + stream.close(); + assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempFile)); + assertTrue(traceOutput.contains("Closed " + tempFile)); + }*/ + private static FileRecord findFileRecord(File file) { for (Record record : Listener.getCurrentOpenFiles()) { if (record instanceof FileRecord) { From 4d847e806a421554a5c4b988b5c0493f84627fbd Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 09:58:57 +0100 Subject: [PATCH 15/20] Add initial README to explain a bit how to develop for this project --- README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..b9c4bb6 --- /dev/null +++ b/README.md @@ -0,0 +1,26 @@ +A Java agent which detects file handle leaks. + +See http://file-leak-detector.kohsuke.org/ for usage description + +# Development + +## Implementation details + +This project uses the JVM support for instrumenting Java classes during startup. + +It adds code to various places where files or sockets are opened and closed to +print out which file handles have not been closed correctly. + +## How to build + + mvn package + +The resulting package will be at `target/file-leak-detector-1.-SNAPSHOT-jar-with-dependencies.jar` + +## How to run integration tests + + mvn verify + +This will run tests in the `org.kohsuke.file_leak_detector.instrumented` package which are +executed with instrumentation via the Java agent being active. + From 8874504eed23723b5ee5e0a626271525e3a62c25 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 21:40:31 +0100 Subject: [PATCH 16/20] Add support for Files.newDirectoryStream, closes #35 --- .../kohsuke/file_leak_detector/AgentMain.java | 25 ++++++- .../kohsuke/file_leak_detector/Listener.java | 5 ++ .../file_leak_detector/AgentMainTest.java | 2 + .../instrumented/FileDemo.java | 71 +++++++++++++++++++ 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java index 97b7a35..6d13ca5 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java +++ b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java @@ -25,6 +25,7 @@ import java.nio.channels.spi.AbstractInterruptibleChannel; import java.nio.channels.spi.AbstractSelectableChannel; import java.nio.channels.spi.AbstractSelector; +import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; @@ -150,6 +151,8 @@ public void run() { addIfFound(classes, "sun/nio/ch/SocketChannelImpl"); addIfFound(classes, "java/net/AbstractPlainSocketImpl"); + addIfFound(classes, "sun/nio/fs/UnixDirectoryStream"); + addIfFound(classes, "sun/nio/fs/UnixSecureDirectoryStream"); instrumentation.retransformClasses(classes.toArray(new Class[0])); @@ -243,11 +246,21 @@ static List createSpec() { new ReturnFromStaticMethodInterceptor("open", "(Ljava/nio/file/Path;Ljava/util/Set;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/channels/FileChannel;", 4, "open_filechannel", FileChannel.class, Path.class)), /* - SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) + * Detect instances opened via static methods in class java.nio.file.Files */ new ClassTransformSpec(Files.class, + // SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) new ReturnFromStaticMethodInterceptor("newByteChannel", - "(Ljava/nio/file/Path;Ljava/util/Set;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/channels/SeekableByteChannel;", 4, "open_filechannel", SeekableByteChannel.class, Path.class)), + "(Ljava/nio/file/Path;Ljava/util/Set;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/channels/SeekableByteChannel;", 4, "open_filechannel", SeekableByteChannel.class, Path.class), + // DirectoryStream newDirectoryStream(Path dir) + new ReturnFromStaticMethodInterceptor("newDirectoryStream", + "(Ljava/nio/file/Path;)Ljava/nio/file/DirectoryStream;", 2, "open_directorystream", DirectoryStream.class, Path.class), + // DirectoryStream newDirectoryStream(Path dir, String glob) + new ReturnFromStaticMethodInterceptor("newDirectoryStream", + "(Ljava/nio/file/Path;Ljava/lang/String;)Ljava/nio/file/DirectoryStream;", 6, "open_directorystream", DirectoryStream.class, Path.class), + // DirectoryStream newDirectoryStream(Path dir, DirectoryStream.Filter filter) + new ReturnFromStaticMethodInterceptor("newDirectoryStream", + "(Ljava/nio/file/Path;Ljava/nio/file/DirectoryStream$Filter;)Ljava/nio/file/DirectoryStream;", 3, "open_directorystream", DirectoryStream.class, Path.class)), /* * Detect new Pipes */ @@ -258,6 +271,14 @@ SeekableByteChannel newByteChannel(Path path, Set options, */ new ClassTransformSpec(AbstractInterruptibleChannel.class, new CloseInterceptor("close")), + /* + * We need to see closing of DirectoryStream instances, + * however they are OS-specific, so we need to list them via String-name + */ + new ClassTransformSpec("sun/nio/fs/UnixDirectoryStream", + new CloseInterceptor("close")), + new ClassTransformSpec("sun/nio/fs/UnixSecureDirectoryStream", + new CloseInterceptor("close")), /** * Detect selectors, which may open native pipes and anonymous inodes for event polling. diff --git a/src/main/java/org/kohsuke/file_leak_detector/Listener.java b/src/main/java/org/kohsuke/file_leak_detector/Listener.java index 715d0fa..d230ab2 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/Listener.java +++ b/src/main/java/org/kohsuke/file_leak_detector/Listener.java @@ -18,6 +18,7 @@ import java.nio.channels.SeekableByteChannel; import java.nio.channels.Selector; import java.nio.channels.SocketChannel; +import java.nio.file.DirectoryStream; import java.nio.file.Path; import java.util.ArrayList; import java.util.Date; @@ -314,6 +315,10 @@ public static synchronized void open_filechannel(SeekableByteChannel byteChannel open(byteChannel, path.toFile()); } + public static synchronized void open_directorystream(DirectoryStream directoryStream, Path path) { + open(directoryStream, path.toFile()); + } + public static synchronized void openSelector(Object _this) { if (_this instanceof Selector) { put(_this, new SelectorRecord((Selector)_this)); diff --git a/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java b/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java index ee6b979..4ee17be 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java +++ b/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java @@ -61,6 +61,8 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable { // the following two are not available in all JVMs seenClasses.remove("sun/nio/ch/SocketChannelImpl"); seenClasses.remove("java/net/AbstractPlainSocketImpl"); + seenClasses.remove("sun/nio/fs/UnixDirectoryStream"); + seenClasses.remove("sun/nio/fs/UnixSecureDirectoryStream"); assertTrue("Had classes in the spec which were not instrumented: " + seenClasses, seenClasses.isEmpty()); diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java index 3212257..f8e7112 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java @@ -10,7 +10,9 @@ import java.io.StringWriter; import java.nio.channels.FileChannel; import java.nio.channels.SeekableByteChannel; +import java.nio.file.DirectoryStream; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.StandardOpenOption; import org.junit.After; @@ -88,6 +90,75 @@ public void openCloseFilesNewByteChannel() throws Exception { assertTrue(traceOutput.contains("Closed " + tempFile)); } + @Test + public void openCloseFilesNewDirectoryStream() throws Exception { + assertTrue(tempFile.delete()); + assertTrue(tempFile.mkdirs()); + + DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath()); + assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + + stream.close(); + assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempFile)); + assertTrue(traceOutput.contains("Closed " + tempFile)); + } + + @Test + public void openCloseFilesNewDirectoryStreamWithStarGlob() throws Exception { + assertTrue(tempFile.delete()); + assertTrue(tempFile.mkdirs()); + + DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath(), "*"); + assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + + stream.close(); + assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempFile)); + assertTrue(traceOutput.contains("Closed " + tempFile)); + } + + @Test + public void openCloseFilesNewDirectoryStreamWithNonStarGlob() throws Exception { + assertTrue(tempFile.delete()); + assertTrue(tempFile.mkdirs()); + + DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath(), "my*test*glob"); + assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + + stream.close(); + assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempFile)); + assertTrue(traceOutput.contains("Closed " + tempFile)); + } + + @Test + public void openCloseFilesNewDirectoryStreamWithFilter() throws Exception { + assertTrue(tempFile.delete()); + assertTrue(tempFile.mkdirs()); + + DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath(), new DirectoryStream.Filter() { + @Override + public boolean accept(Path entry) { + return true; + } + }); + assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + + stream.close(); + assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempFile)); + assertTrue(traceOutput.contains("Closed " + tempFile)); + } + /* This is only available in Java 8 and newer... @Test public void openCloseFileLines() throws Exception { From 31c4e6aba21eb0558867e78cbefd0fc3456fc6f2 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 3 Jan 2020 21:56:53 +0100 Subject: [PATCH 17/20] Also verify that the correct type of object is registered as 'marker' by the instrumentation --- .../instrumented/FileDemo.java | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java index f8e7112..0fdc09c 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java @@ -19,6 +19,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.kohsuke.file_leak_detector.ActivityListener; import org.kohsuke.file_leak_detector.Listener; import org.kohsuke.file_leak_detector.Listener.FileRecord; import org.kohsuke.file_leak_detector.Listener.Record; @@ -30,6 +31,29 @@ public class FileDemo { private static StringWriter output = new StringWriter(); private File tempFile; + private Object obj; + + private final ActivityListener listener = new ActivityListener() { + @Override + public void open(Object obj, File file) { + FileDemo.this.obj = obj; + } + + @Override + public void openSocket(Object obj) { + FileDemo.this.obj = obj; + } + + @Override + public void close(Object obj) { + FileDemo.this.obj = obj; + } + + @Override + public void fd_open(Object obj) { + FileDemo.this.obj = obj; + } + }; @BeforeClass public static void setup() { @@ -38,6 +62,16 @@ public static void setup() { Listener.TRACE = new PrintWriter(output); } + @Before + public void registerListener() { + ActivityListener.LIST.add(listener); + } + + @After + public void unregisterListener() { + ActivityListener.LIST.remove(listener); + } + @Before public void prepareOutput() throws Exception { output.getBuffer().setLength(0); @@ -54,6 +88,9 @@ public void openCloseFile() throws Exception { FileInputStream in = new FileInputStream(tempFile); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof FileInputStream); + in.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -67,6 +104,9 @@ public void openCloseFileChannel() throws Exception { FileChannel fileChannel = FileChannel.open(tempFile.toPath(), StandardOpenOption.APPEND); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof FileChannel); + fileChannel.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -82,6 +122,9 @@ public void openCloseFilesNewByteChannel() throws Exception { SeekableByteChannel fileChannel = Files.newByteChannel(tempFile.toPath(), StandardOpenOption.APPEND); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof SeekableByteChannel); + fileChannel.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -98,6 +141,9 @@ public void openCloseFilesNewDirectoryStream() throws Exception { DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath()); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof DirectoryStream); + stream.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -114,6 +160,9 @@ public void openCloseFilesNewDirectoryStreamWithStarGlob() throws Exception { DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath(), "*"); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof DirectoryStream); + stream.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -130,6 +179,9 @@ public void openCloseFilesNewDirectoryStreamWithNonStarGlob() throws Exception { DirectoryStream stream = Files.newDirectoryStream(tempFile.toPath(), "my*test*glob"); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof DirectoryStream); + stream.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -151,6 +203,9 @@ public boolean accept(Path entry) { }); assertNotNull("No file record for file=" + tempFile + " found", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof DirectoryStream); + stream.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); @@ -168,6 +223,9 @@ public void openCloseFileLines() throws Exception { stream.close(); assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof Stream); + String traceOutput = output.toString(); assertTrue(traceOutput.contains("Opened " + tempFile)); assertTrue(traceOutput.contains("Closed " + tempFile)); From b416d301b4f77f7d287458ff8faaa69b1784ba4e Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 17 Sep 2020 21:03:41 +0200 Subject: [PATCH 18/20] Update minimum Java to 1.8 to enable some more tests --- pom.xml | 8 ++++---- .../kohsuke/file_leak_detector/instrumented/FileDemo.java | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 8f389f5..25e24ba 100644 --- a/pom.xml +++ b/pom.xml @@ -31,8 +31,8 @@ maven-compiler-plugin - 1.6 - 1.6 + 1.8 + 1.8 @@ -120,8 +120,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.7 - 1.7 + 8 + 8 diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java index 0fdc09c..40c400e 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java @@ -14,6 +14,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.stream.Stream; import org.junit.After; import org.junit.Before; @@ -29,7 +30,7 @@ * @author Andreas Dangel */ public class FileDemo { - private static StringWriter output = new StringWriter(); + private static final StringWriter output = new StringWriter(); private File tempFile; private Object obj; @@ -214,7 +215,6 @@ public boolean accept(Path entry) { assertTrue(traceOutput.contains("Closed " + tempFile)); } - /* This is only available in Java 8 and newer... @Test public void openCloseFileLines() throws Exception { Stream stream = Files.lines(tempFile.toPath()); @@ -224,12 +224,12 @@ public void openCloseFileLines() throws Exception { assertNull("File record for file=" + tempFile + " not removed", findFileRecord(tempFile)); assertTrue("Did not have the expected type of 'marker' object: " + obj, - obj instanceof Stream); + obj instanceof SeekableByteChannel); String traceOutput = output.toString(); assertTrue(traceOutput.contains("Opened " + tempFile)); assertTrue(traceOutput.contains("Closed " + tempFile)); - }*/ + } private static FileRecord findFileRecord(File file) { for (Record record : Listener.getCurrentOpenFiles()) { From 600830cd4b3c11cd5a66f972752e7337de410b15 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 17 Sep 2020 21:07:37 +0200 Subject: [PATCH 19/20] Add failing test which uses a custom filesystem which LuceneTestCase does This triggers a problem because we record open in Files.newDirectoryStream, but close in a different place, so a replaced FileSystemProvider actually uses a different object then. --- .../instrumented/LuceneTestCase.java | 648 ++++++++++++++++++ 1 file changed, 648 insertions(+) create mode 100644 src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java new file mode 100644 index 0000000..a9dfdee --- /dev/null +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java @@ -0,0 +1,648 @@ +package org.kohsuke.file_leak_detector.instrumented; + +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.kohsuke.file_leak_detector.ActivityListener; +import org.kohsuke.file_leak_detector.Listener; +import org.kohsuke.file_leak_detector.Listener.FileRecord; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.URI; +import java.nio.channels.AsynchronousFileChannel; +import java.nio.channels.FileChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.AccessMode; +import java.nio.file.CopyOption; +import java.nio.file.DirectoryStream; +import java.nio.file.DirectoryStream.Filter; +import java.nio.file.FileStore; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.ProviderMismatchException; +import java.nio.file.WatchEvent; +import java.nio.file.WatchKey; +import java.nio.file.WatchService; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.UserPrincipalLookupService; +import java.nio.file.spi.FileSystemProvider; +import java.util.Iterator; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.function.Consumer; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class LuceneTestCase { + private static final StringWriter output = new StringWriter(); + private Path tempDir; + private Object obj; + private FileSystem fileSystem; + + private final ActivityListener listener = new ActivityListener() { + @Override + public void open(Object obj, File file) { + LuceneTestCase.this.obj = obj; + } + + @Override + public void openSocket(Object obj) { + LuceneTestCase.this.obj = obj; + } + + @Override + public void close(Object obj) { + LuceneTestCase.this.obj = obj; + } + + @Override + public void fd_open(Object obj) { + LuceneTestCase.this.obj = obj; + } + }; + + @BeforeClass + public static void setup() { + assertTrue("This test expects the Java Agent to be installed via commandline options", + Listener.isAgentInstalled()); + Listener.TRACE = new PrintWriter(output); + } + + @Before + public void registerListener() { + ActivityListener.LIST.add(listener); + } + + @After + public void unregisterListener() { + ActivityListener.LIST.remove(listener); + } + + @Before + public void prepareOutput() throws Exception { + FileSystem fs = FileSystems.getDefault(); + fs = new FilterFileSystemProvider("extras://", fs) {}.getFileSystem(null); + fileSystem = fs.provider().getFileSystem(URI.create("file:///")); + + tempDir = fileSystem.getPath(System.getProperty("java.io.tmpdir")); + + Files.createDirectories(tempDir); + } + + @Test + public void testConstants() throws Throwable { + DirectoryStream directoryStream = Files.newDirectoryStream(tempDir); + + assertNotNull("No file record for file=" + tempDir + " found", findFileRecord(tempDir.toFile())); + + assertTrue("Did not have the expected type of 'marker' object: " + obj, + obj instanceof DirectoryStream); + + directoryStream.close(); + + assertNull("File record for file=" + tempDir + " not removed", findFileRecord(tempDir.toFile())); + + fileSystem.close(); + + String traceOutput = output.toString(); + assertTrue(traceOutput.contains("Opened " + tempDir)); + assertTrue(traceOutput.contains("Closed " + tempDir)); + } + + public abstract class FilterFileSystemProvider extends FileSystemProvider { + protected final FileSystemProvider delegate; + protected FileSystem fileSystem; + protected final String scheme; + + public FilterFileSystemProvider(String scheme, FileSystem delegateInstance) { + this.scheme = Objects.requireNonNull(scheme); + Objects.requireNonNull(delegateInstance); + this.delegate = delegateInstance.provider(); + this.fileSystem = new FilterFileSystem(this, delegateInstance); + } + + @Override + public String getScheme() { + return scheme; + } + + @Override + public FileSystem newFileSystem(URI uri, Map env) { + if (fileSystem == null) { + throw new IllegalStateException("subclass did not initialize singleton filesystem"); + } + return fileSystem; + } + + @Override + public FileSystem newFileSystem(Path path, Map env) { + if (fileSystem == null) { + throw new IllegalStateException("subclass did not initialize singleton filesystem"); + } + return fileSystem; + } + + @Override + public FileSystem getFileSystem(URI uri) { + if (fileSystem == null) { + throw new IllegalStateException("subclass did not initialize singleton filesystem"); + } + return fileSystem; + } + + + @Override + public Path getPath( URI uri) { + if (fileSystem == null) { + throw new IllegalStateException("subclass did not initialize singleton filesystem"); + } + Path path = delegate.getPath(uri); + return new FilterPath(path, fileSystem); + } + + @Override + public void createDirectory(Path dir, FileAttribute... attrs) throws IOException { + delegate.createDirectory(toDelegate(dir), attrs); + } + + @Override + public void delete(Path path) throws IOException { + delegate.delete(toDelegate(path)); + } + + @Override + public void copy(Path source, Path target, CopyOption... options) throws IOException { + delegate.copy(toDelegate(source), toDelegate(target), options); + } + + @Override + public void move(Path source, Path target, CopyOption... options) throws IOException { + delegate.move(toDelegate(source), toDelegate(target), options); + } + + @Override + public boolean isSameFile(Path path, Path path2) throws IOException { + return delegate.isSameFile(toDelegate(path), toDelegate(path2)); + } + + @Override + public boolean isHidden(Path path) throws IOException { + return delegate.isHidden(toDelegate(path)); + } + + @Override + public FileStore getFileStore(Path path) throws IOException { + return delegate.getFileStore(toDelegate(path)); + } + + @Override + public void checkAccess(Path path, AccessMode... modes) throws IOException { + delegate.checkAccess(toDelegate(path), modes); + } + + @Override + public V getFileAttributeView(Path path, Class type, LinkOption... options) { + return delegate.getFileAttributeView(toDelegate(path), type, options); + } + + @Override + public A readAttributes(Path path, Class type, LinkOption... options) throws IOException { + return delegate.readAttributes(toDelegate(path), type, options); + } + + @Override + public Map readAttributes(Path path, String attributes, LinkOption... options) throws IOException { + return delegate.readAttributes(toDelegate(path), attributes, options); + } + + @Override + public void setAttribute(Path path, String attribute, Object value, LinkOption... options) throws IOException { + delegate.setAttribute(toDelegate(path), attribute, value, options); + } + + @Override + public InputStream newInputStream(Path path, OpenOption... options) throws IOException { + return delegate.newInputStream(toDelegate(path), options); + } + + @Override + public OutputStream newOutputStream(Path path, OpenOption... options) throws IOException { + return delegate.newOutputStream(toDelegate(path), options); + } + + @Override + public FileChannel newFileChannel(Path path, Set options, FileAttribute... attrs) throws IOException { + return delegate.newFileChannel(toDelegate(path), options, attrs); + } + + @Override + public AsynchronousFileChannel newAsynchronousFileChannel(Path path, Set options, ExecutorService executor, FileAttribute... attrs) throws IOException { + return delegate.newAsynchronousFileChannel(toDelegate(path), options, executor, attrs); + } + + @Override + public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) throws IOException { + return delegate.newByteChannel(toDelegate(path), options, attrs); + } + + @Override + public DirectoryStream newDirectoryStream(Path dir, final Filter filter) throws IOException { + return new FilterDirectoryStream(delegate.newDirectoryStream(toDelegate(dir), filter), fileSystem); + } + + @Override + public void createSymbolicLink(Path link, Path target, FileAttribute... attrs) throws IOException { + delegate.createSymbolicLink(toDelegate(link), toDelegate(target), attrs); + } + + @Override + public void createLink(Path link, Path existing) throws IOException { + delegate.createLink(toDelegate(link), toDelegate(existing)); + } + + @Override + public boolean deleteIfExists(Path path) throws IOException { + return delegate.deleteIfExists(toDelegate(path)); + } + + @Override + public Path readSymbolicLink(Path link) throws IOException { + return delegate.readSymbolicLink(toDelegate(link)); + } + + protected Path toDelegate(Path path) { + if (path instanceof FilterPath) { + FilterPath fp = (FilterPath) path; + if (fp.fileSystem != fileSystem) { + throw new ProviderMismatchException("mismatch, expected: " + fileSystem.provider().getClass() + ", got: " + fp.fileSystem.provider().getClass()); + } + return fp.delegate; + } else { + throw new ProviderMismatchException("mismatch, expected: FilterPath, got: " + path.getClass()); + } + } + + /** + * Override to trigger some behavior when the filesystem is closed. + *

+ * This is always called for each FilterFileSystemProvider in the chain. + */ + protected void onClose() { + } + + @Override + public String toString() { + return getClass().getSimpleName() + "(" + delegate + ")"; + } + } + + public class FilterFileSystem extends FileSystem { + + protected final FilterFileSystemProvider parent; + + protected final FileSystem delegate; + + public FilterFileSystem(FilterFileSystemProvider parent, FileSystem delegate) { + this.parent = Objects.requireNonNull(parent); + this.delegate = Objects.requireNonNull(delegate); + } + + @Override + public FileSystemProvider provider() { + return parent; + } + + @Override + public void close() throws IOException { + if (delegate == FileSystems.getDefault()) { + // you can't close the default provider! + parent.onClose(); + } else { + //noinspection unused + try (FileSystem d = delegate) { + parent.onClose(); + } + } + } + + @Override + public boolean isOpen() { + return delegate.isOpen(); + } + + @Override + public boolean isReadOnly() { + return delegate.isReadOnly(); + } + + @Override + public String getSeparator() { + return delegate.getSeparator(); + } + + @Override + public Iterable getRootDirectories() { + throw new UnsupportedOperationException(); + } + + @Override + public Iterable getFileStores() { + throw new UnsupportedOperationException(); + } + + @Override + public Set supportedFileAttributeViews() { + return delegate.supportedFileAttributeViews(); + } + + + @Override + public Path getPath( String first, String... more) { + return new FilterPath(delegate.getPath(first, more), this); + } + + @Override + public PathMatcher getPathMatcher(String syntaxAndPattern) { + final PathMatcher matcher = delegate.getPathMatcher(syntaxAndPattern); + return path -> { + if (path instanceof FilterPath) { + return matcher.matches(((FilterPath)path).delegate); + } + return false; + }; + } + + @Override + public UserPrincipalLookupService getUserPrincipalLookupService() { + return delegate.getUserPrincipalLookupService(); + } + + @Override + public WatchService newWatchService() throws IOException { + return delegate.newWatchService(); + } + } + + public static class FilterPath implements Path { + + @Override + public void forEach(Consumer action) { + + } + + protected final Path delegate; + + protected final FileSystem fileSystem; + + public FilterPath(Path delegate, FileSystem fileSystem) { + this.delegate = delegate; + this.fileSystem = fileSystem; + } + + + @Override + public FileSystem getFileSystem() { + return fileSystem; + } + + @Override + public boolean isAbsolute() { + return delegate.isAbsolute(); + } + + @Override + public Path getRoot() { + Path root = delegate.getRoot(); + if (root == null) { + return null; + } + return wrap(root); + } + + @Override + public Path getFileName() { + Path fileName = delegate.getFileName(); + if (fileName == null) { + return null; + } + return wrap(fileName); + } + + @Override + public Path getParent() { + Path parent = delegate.getParent(); + if (parent == null) { + return null; + } + return wrap(parent); + } + + @Override + public int getNameCount() { + return delegate.getNameCount(); + } + + + @Override + public Path getName(int index) { + return wrap(delegate.getName(index)); + } + + + @Override + public Path subpath(int beginIndex, int endIndex) { + return wrap(delegate.subpath(beginIndex, endIndex)); + } + + @Override + public boolean startsWith( Path other) { + return delegate.startsWith(toDelegate(other)); + } + + @Override + public boolean startsWith( String other) { + return delegate.startsWith(other); + } + + @Override + public boolean endsWith( Path other) { + return delegate.endsWith(toDelegate(other)); + } + + @Override + public boolean endsWith( String other) { + return delegate.startsWith(other); + } + + @Override + public Path normalize() { + return wrap(delegate.normalize()); + } + + + @Override + public Path resolve( Path other) { + return wrap(delegate.resolve(toDelegate(other))); + } + + + @Override + public Path resolve( String other) { + return wrap(delegate.resolve(other)); + } + + + @Override + public Path resolveSibling( Path other) { + return wrap(delegate.resolveSibling(toDelegate(other))); + } + + + @Override + public Path resolveSibling( String other) { + return wrap(delegate.resolveSibling(other)); + } + + + @Override + public Path relativize( Path other) { + return wrap(delegate.relativize(toDelegate(other))); + } + + @Override + public URI toUri() { + return delegate.toUri(); + } + + @Override + public String toString() { + return delegate.toString(); + } + + + @Override + public Path toAbsolutePath() { + return wrap(delegate.toAbsolutePath()); + } + + + @Override + public Path toRealPath( LinkOption... options) throws IOException { + return wrap(delegate.toRealPath(options)); + } + + + @Override + public File toFile() { + return delegate.toFile(); + } + + @Override + public WatchKey register(WatchService watcher, WatchEvent.Kind[] events, WatchEvent.Modifier... modifiers) throws IOException { + return delegate.register(watcher, events, modifiers); + } + + @Override + public WatchKey register(WatchService watcher, WatchEvent.Kind... events) throws IOException { + return delegate.register(watcher, events); + } + + + @Override + public Iterator iterator() { + throw new UnsupportedOperationException(); + } + + @Override + public int compareTo(Path other) { + return delegate.compareTo(toDelegate(other)); + } + + @Override + public int hashCode() { + return delegate.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + FilterPath other = (FilterPath) obj; + if (delegate == null) { + if (other.delegate != null) return false; + } else if (!delegate.equals(other.delegate)) return false; + if (fileSystem == null) { + return other.fileSystem == null; + } else + return fileSystem.equals(other.fileSystem); + } + + protected Path wrap(Path other) { + return new FilterPath(other, fileSystem); + } + + protected Path toDelegate(Path path) { + if (path instanceof FilterPath) { + FilterPath fp = (FilterPath) path; + if (fp.fileSystem != fileSystem) { + throw new ProviderMismatchException("mismatch, expected: " + fileSystem.provider().getClass() + ", got: " + fp.fileSystem.provider().getClass()); + } + return fp.delegate; + } else { + throw new ProviderMismatchException("mismatch, expected: FilterPath, got: " + path.getClass()); + } + } + } + + public static class FilterDirectoryStream implements DirectoryStream { + + protected final DirectoryStream delegate; + + protected final FileSystem fileSystem; + + public FilterDirectoryStream(DirectoryStream delegate, FileSystem fileSystem) { + this.delegate = Objects.requireNonNull(delegate); + this.fileSystem = Objects.requireNonNull(fileSystem); + } + + @Override + public void close() throws IOException { + delegate.close(); + } + + @Override + public Iterator iterator() { + throw new UnsupportedOperationException(); + } + } + + private static FileRecord findFileRecord(File file) { + for (Listener.Record record : Listener.getCurrentOpenFiles()) { + if (record instanceof FileRecord) { + FileRecord fileRecord = (FileRecord) record; + if (fileRecord.file == file || fileRecord.file.getName().equals(file.getName())) { + return fileRecord; + } + } + } + return null; + } +} From ba46bd18edfc8973d3e3618085531b67e7b40e98 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Wed, 6 Oct 2021 22:08:02 +0200 Subject: [PATCH 20/20] Tried to add some more functionality, but did not work This should probably be reverted anyway... --- pom.xml | 4 +- .../kohsuke/file_leak_detector/AgentMain.java | 5 ++ .../transform/TransformerImpl.java | 63 +++++++++++++++++-- .../file_leak_detector/AgentMainTest.java | 2 +- .../instrumented/LuceneTestCase.java | 3 +- 5 files changed, 69 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 25e24ba..d486d7e 100644 --- a/pom.xml +++ b/pom.xml @@ -138,13 +138,13 @@ args4j 2.0.16 - + commons-io commons-io diff --git a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java index 6d13ca5..205dc99 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java +++ b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java @@ -162,6 +162,9 @@ public void run() { // AbstractInterruptibleChannel.class, // ServerSocket.class); + /*instrumentation.addTransformer(new InterfaceTransformerImpl(createSpec()),true); + instrumentation.retransformClasses(classes.toArray(new Class[0]));*/ + if (serverPort>=0) runHttpServer(serverPort); } @@ -279,6 +282,8 @@ static List createSpec() { new CloseInterceptor("close")), new ClassTransformSpec("sun/nio/fs/UnixSecureDirectoryStream", new CloseInterceptor("close")), + /*new ClassTransformSpec("java/nio/file/DirectoryStream", + new CloseInterceptor("close")),*/ /** * Detect selectors, which may open native pipes and anonymous inodes for event polling. diff --git a/src/main/java/org/kohsuke/file_leak_detector/transform/TransformerImpl.java b/src/main/java/org/kohsuke/file_leak_detector/transform/TransformerImpl.java index 4312cb3..5ad5d76 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/transform/TransformerImpl.java +++ b/src/main/java/org/kohsuke/file_leak_detector/transform/TransformerImpl.java @@ -4,6 +4,8 @@ import org.kohsuke.asm6.ClassVisitor; import org.kohsuke.asm6.ClassWriter; import org.kohsuke.asm6.MethodVisitor; +import org.kohsuke.file_leak_detector.AgentMain; +import org.kohsuke.file_leak_detector.Listener; import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.IllegalClassFormatException; @@ -30,14 +32,66 @@ public TransformerImpl(Collection specs) { public byte[] transform(ClassLoader loader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException { return transform(className,classfileBuffer); } - + public byte[] transform(String className, byte[] classfileBuffer) { - final ClassTransformSpec cs = specs.get(className); - if(cs==null) + //System.out.println("Loading " + className); + + ClassTransformSpec csa = specs.get(className); + + if(csa ==null) { + /*if (className.contains("DirectoryStream")) { + //classfileBuffer = transformInterfaces(className, classfileBuffer); + return transformBytes(className, classfileBuffer, + new ClassTransformSpec(className, + new CloseInterceptor("close"))); + }*/ + return classfileBuffer; + } + + return transformBytes(className, classfileBuffer, csa); + } + + /** + * Intercepts a void method used to close a handle and calls {@link Listener#close(Object)} in the end. + */ + private static class CloseInterceptor extends MethodAppender { + public CloseInterceptor(String methodName) { + super(methodName, "()V"); + } + protected void append(CodeGenerator g) { + g.invokeAppStatic(Listener.class,"close", + new Class[]{Object.class}, + new int[]{0}); + } + } + + /*public byte[] transformInterfaces(String className, byte[] classfileBuffer) { + // try to look if this class implements an interface that is transformed + try { + Class aClass = Class.forName(className.replace("/", ".")); + for (Class anInterface : aClass.getInterfaces()) { + String interfaceName = anInterface.getName().replace(".", "/"); + System.out.println("Looking at " + className + ": " + interfaceName); + ClassTransformSpec classTransformSpecInterface = specs.get(interfaceName); + if (classTransformSpecInterface != null) { + System.out.println("Found interface-spec for " + className + ": " + interfaceName); + return transformBytes(className, classfileBuffer, classTransformSpecInterface); + } + } + } catch (ClassNotFoundException e) { + System.out.println("Failed to load class " + className); + e.printStackTrace(); + } + + return classfileBuffer; + }*/ + + protected byte[] transformBytes(String className, byte[] classfileBuffer, ClassTransformSpec cs) { ClassReader cr = new ClassReader(classfileBuffer); ClassWriter cw = new ClassWriter(/*ClassWriter.COMPUTE_FRAMES|*/ClassWriter.COMPUTE_MAXS); + cr.accept(new ClassVisitor(ASM6,cw) { @Override public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { @@ -47,11 +101,12 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si if(ms==null) ms = cs.methodSpecs.get(name+"*"); if(ms==null) return base; + //System.out.println("Transforming method " + name + " of " + cs.name); return ms.newAdapter(base,access,name,desc,signature,exceptions); } }, SKIP_FRAMES); -// System.out.println("Transforming "+className); + //System.out.println("Transformed "+ className); return cw.toByteArray(); } } diff --git a/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java b/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java index 4ee17be..a2ee7b2 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java +++ b/src/test/java/org/kohsuke/file_leak_detector/AgentMainTest.java @@ -58,7 +58,7 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable { verify(instrumentation, times(1)).addTransformer((ClassFileTransformer) any(), anyBoolean()); verify(instrumentation, times(1)).retransformClasses((Class) any()); - // the following two are not available in all JVMs + // the following are not available in all JVMs seenClasses.remove("sun/nio/ch/SocketChannelImpl"); seenClasses.remove("java/net/AbstractPlainSocketImpl"); seenClasses.remove("sun/nio/fs/UnixDirectoryStream"); diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java index a9dfdee..89d6c55 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/LuceneTestCase.java @@ -117,7 +117,8 @@ public void testConstants() throws Throwable { directoryStream.close(); - assertNull("File record for file=" + tempDir + " not removed", findFileRecord(tempDir.toFile())); + assertNull("File record for file=" + tempDir + " not removed, having class: " + directoryStream.getClass(), + findFileRecord(tempDir.toFile())); fileSystem.close();