Various smaller adjustments

.gitignore

@@ -5,3 +5,4 @@
 /.idea/*/*
 /.idea/*
 /.idea
+/file-leak-detector.iml

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.<version>-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.
+

pom.xml

@@ -7,7 +7,7 @@
   </parent>
   <groupId>org.kohsuke</groupId>
   <artifactId>file-leak-detector</artifactId>
-  <version>1.13-SNAPSHOT</version>
+  <version>1.14-SNAPSHOT</version>
   <name>File Leak Detector</name>
 
   <url>http://${project.artifactId}.kohsuke.org/</url>
@@ -31,8 +31,8 @@
       <plugin>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>1.6</source>
-          <target>1.6</target>
+          <source>1.8</source>
+          <target>1.8</target>
         </configuration>
       </plugin>
       <plugin>
@@ -53,6 +53,7 @@
             </includes>
             <excludes>
               <exclude>**/TransformerTest.java</exclude>
+              <exclude>**/AgentMainTest.java</exclude>
             </excludes>
             <argLine>
               -javaagent:"${project.build.directory}/file-leak-detector-${project.version}-jar-with-dependencies.jar"
@@ -67,9 +68,8 @@
               </goals>
               <configuration>
                 <includes>
-                  <include>
-                    **/TransformerTest.java
-                  </include>
+                  <include>**/TransformerTest.java</include>
+                  <include>**/AgentMainTest.java</include>
                 </includes>
                 <excludes>
                   <exclude>**/instrumented/*.java</exclude>
@@ -116,6 +116,14 @@
         <configuration>
         </configuration>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <source>8</source>
+          <target>8</target>
+        </configuration>
+      </plugin>
     </plugins>
   </build>
 
@@ -130,13 +138,13 @@
       <artifactId>args4j</artifactId>
       <version>2.0.16</version>
     </dependency>
-    <dependency>
+    <!--dependency>
       <groupId>com.sun</groupId>
       <artifactId>tools</artifactId>
       <version>6.0</version>
       <scope>system</scope>
       <systemPath>${toolsjar}</systemPath>
-    </dependency>
+    </dependency-->
     <dependency>
       <groupId>commons-io</groupId>
       <artifactId>commons-io</artifactId>
@@ -146,7 +154,13 @@
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
-      <version>4.9</version>
+      <version>4.12</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>3.2.4</version>
       <scope>test</scope>
     </dependency>
   </dependencies>

src/main/java/org/kohsuke/file_leak_detector/AgentMain.java

@@ -21,9 +21,12 @@
 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.DirectoryStream;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.List;
@@ -52,10 +55,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;
@@ -133,8 +136,8 @@ public void run() {
 
         Listener.AGENT_INSTALLED = true;
         instrumentation.addTransformer(new TransformerImpl(createSpec()),true);
-        
-        instrumentation.retransformClasses(
+
+        List<Class> classes = Arrays.asList(new Class[] {
                 FileInputStream.class,
                 FileOutputStream.class,
                 RandomAccessFile.class,
@@ -143,19 +146,37 @@ 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");
+        addIfFound(classes, "sun/nio/fs/UnixDirectoryStream");
+        addIfFound(classes, "sun/nio/fs/UnixSecureDirectoryStream");
+
+        instrumentation.retransformClasses(classes.toArray(new Class[0]));
 
 
 //                Socket.class,
 //                SocketChannel.class,
 //                AbstractInterruptibleChannel.class,
 //                ServerSocket.class);
 
+        /*instrumentation.addTransformer(new InterfaceTransformerImpl(createSpec()),true);
+        instrumentation.retransformClasses(classes.toArray(new Class[0]));*/
+
         if (serverPort>=0)
             runHttpServer(serverPort);
     }
 
+    private static void addIfFound(List<Class> 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));
@@ -227,6 +248,22 @@ static List<ClassTransformSpec> 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)),
+            /*
+             * Detect instances opened via static methods in class java.nio.file.Files
+             */
+            new ClassTransformSpec(Files.class,
+                    // SeekableByteChannel newByteChannel(Path path, Set<? extends OpenOption> 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),
+                    // DirectoryStream<Path> newDirectoryStream(Path dir)
+                    new ReturnFromStaticMethodInterceptor("newDirectoryStream",
+                            "(Ljava/nio/file/Path;)Ljava/nio/file/DirectoryStream;", 2, "open_directorystream", DirectoryStream.class, Path.class),
+                    // DirectoryStream<Path> 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<Path> newDirectoryStream(Path dir, DirectoryStream.Filter<? super Path> 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
              */
@@ -237,6 +274,16 @@ static List<ClassTransformSpec> createSpec() {
              */
             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")),
+            /*new ClassTransformSpec("java/nio/file/DirectoryStream",
+                    new CloseInterceptor("close")),*/
 
             /**
              * Detect selectors, which may open native pipes and anonymous inodes for event polling.
@@ -249,7 +296,7 @@ static List<ClassTransformSpec> 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",
@@ -353,6 +400,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 +446,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<? extends Exception> getExpectedException() {
             return IOException.class;
         }
@@ -407,7 +455,7 @@ protected Class<? extends Exception> 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

src/main/java/org/kohsuke/file_leak_detector/Listener.java

@@ -15,8 +15,10 @@
 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.DirectoryStream;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Date;
@@ -28,7 +30,7 @@
 
 /**
  * Intercepted JDK calls land here.
- * 
+ *
  * @author Kohsuke Kawaguchi
  */
 public class Listener {
@@ -60,7 +62,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 +77,7 @@ public boolean exclude() {
                     break;
                 }
 			}
-            
+
             // check the rest
             for (; i < trace.length; i++) {
             	String t = trace[i].toString();
@@ -244,7 +246,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<String> EXCLUDES = new ArrayList<String>();
 
@@ -270,7 +272,7 @@ public void dump(String prefix, PrintWriter ps) {
     public static boolean isAgentInstalled() {
         return AGENT_INSTALLED;
     }
-    
+
     public static synchronized void makeStrong() {
         TABLE = new LinkedHashMap<Object, Record>(TABLE);
     }
@@ -309,6 +311,14 @@ 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 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));
@@ -353,11 +363,11 @@ public static synchronized void openSocket(Object _this) {
             }
         }
     }
-    
+
     public static synchronized List<Record> getCurrentOpenFiles() {
         return new ArrayList<Record>(TABLE.values());
     }
-    
+
     private static synchronized void put(Object _this, Record r) {
     	// handle excludes
     	if(r.exclude()) {
@@ -435,7 +445,7 @@ public static synchronized void outOfDescriptors() {
             tracing = false;
         }
     }
-    
+
     private static String format(long time) {
         try {
             return new Date(time).toString();
@@ -445,7 +455,7 @@ private static String format(long time) {
     }
 
     private static Field SOCKETIMPL_SOCKET,SOCKETIMPL_SERVER_SOCKET;
-    
+
     static {
         try {
             SOCKETIMPL_SOCKET = SocketImpl.class.getDeclaredField("socket");

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);
         }

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<String,MethodTransformSpec> methodSpecs = new HashMap<String,MethodTransformSpec>();
 
-    public ClassTransformSpec(Class clazz, MethodTransformSpec... methodSpecs) {
+    public ClassTransformSpec(Class<?> clazz, MethodTransformSpec... methodSpecs) {
         this(clazz.getName().replace('.', '/'),methodSpecs);
     }