Merge pull request #348 from internetarchive/fixes-leaky-file-handles

Fixes leaky file handles

Author: anjackson

contrib/src/main/java/org/archive/modules/extractor/ExtractorYoutubeDL.java

@@ -117,17 +117,52 @@ public class ExtractorYoutubeDL extends Extractor
     // unnamed toethread-local temporary file
     protected transient ThreadLocal<RandomAccessFile> tempfile = new ThreadLocal<RandomAccessFile>() {
         protected RandomAccessFile initialValue() {
-            File t;
-            try {
-                t = File.createTempFile("ydl", ".json");
-                RandomAccessFile f = new RandomAccessFile(t, "rw");
-                t.delete();
-                return f;
-            } catch (IOException e) {
-                throw new RuntimeException(e);
-            }
+            return null;
         }
     };
+    protected void closeLocalTempFile() {
+        RandomAccessFile localTemp = tempfile.get();
+        if(localTemp == null || !isOpen(localTemp))
+            return; // avoid making a new temp file just to close it immediately
+        try {
+            getLocalTempFile().close();
+	    tempfile.set(null);
+        }
+        catch (Exception e) {
+            logger.log(Level.WARNING, "problem closing ydl temp file " + e);
+        }
+    }
+    protected RandomAccessFile getLocalTempFile() {
+        RandomAccessFile localTemp = tempfile.get();
+        if(localTemp == null || !isOpen(localTemp)) {
+            localTemp = openNewTempFile();
+            tempfile.set(localTemp);
+        }
+	logger.info("Getting youtube-dl temp file ");
+        return localTemp;
+    }
+    protected boolean isOpen(RandomAccessFile f) {
+        try {
+            f.length();
+            return true;
+        }
+        catch (IOException e) {
+	    logger.info("youtube-dl temp file is not open");
+            return false ;
+        }
+    }
+    protected RandomAccessFile openNewTempFile() {
+	logger.info("Opening New youtube-dl temp file ");
+	File t;
+        try {
+            t = File.createTempFile("ydl", ".json");
+            RandomAccessFile f = new RandomAccessFile(t, "rw");
+            t.delete();
+            return f;
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
 
     protected CrawlerLoggerModule crawlerLoggerModule;
     public CrawlerLoggerModule getCrawlerLoggerModule() {
@@ -447,7 +482,7 @@ public String call() throws IOException {
             }
         });
 
-        YoutubeDLResults results = new YoutubeDLResults(tempfile.get());
+        YoutubeDLResults results = new YoutubeDLResults(getLocalTempFile());
 
         try {
             try {
@@ -525,7 +560,14 @@ public boolean shouldBuildRecord(CrawlURI uri) {
         // should build record for containing page, which has an
         // annotation like "youtube-dl:3" (no slash)
         String annotation = findYdlAnnotation(uri);
-        return annotation != null && !annotation.contains("/");
+        boolean shouldBuild = (annotation != null && !annotation.contains("/"));
+
+        // If we processed this uri, then we have an open temp file that won't get closed
+        // for us by the warc writer
+        if(!shouldBuild)
+            closeLocalTempFile();
+
+        return shouldBuild;
     }
 
     @Override
@@ -546,10 +588,10 @@ public WARCRecordInfo buildRecord(CrawlURI curi, URI concurrentTo)
         recordInfo.setMimetype("application/vnd.youtube-dl_formats+json;charset=utf-8");
         recordInfo.setEnforceLength(true);
 
-        tempfile.get().seek(0);
-        InputStream inputStream = Channels.newInputStream(tempfile.get().getChannel());
+        getLocalTempFile().seek(0);
+        InputStream inputStream = Channels.newInputStream(getLocalTempFile().getChannel());
         recordInfo.setContentStream(inputStream);
-        recordInfo.setContentLength(tempfile.get().length());
+        recordInfo.setContentLength(getLocalTempFile().length());
 
         logger.info("built record timestamp=" + timestamp + " url=" + recordInfo.getUrl());
 
@@ -575,7 +617,7 @@ public static void main(String[] args) throws IOException {
         ExtractorYoutubeDL e = new ExtractorYoutubeDL();
 
         FileInputStream in = new FileInputStream("/tmp/ydl-single-video.json");
-        YoutubeDLResults results = new YoutubeDLResults(e.tempfile.get());
+        YoutubeDLResults results = new YoutubeDLResults(e.getLocalTempFile());
         e.streamYdlOutput(in, results);
         System.out.println("video urls: " + results.videoUrls);
         System.out.println("page urls: " + results.pageUrls);
@@ -591,7 +633,7 @@ public static void main(String[] args) throws IOException {
         }
 
         in = new FileInputStream("/tmp/ydl-uncgreensboro-limited.json");
-        results = new YoutubeDLResults(e.tempfile.get());
+        results = new YoutubeDLResults(e.getLocalTempFile());
         e.streamYdlOutput(in, results);
         System.out.println("video urls: " + results.videoUrls);
         System.out.println("page urls: " + results.pageUrls);

modules/src/main/java/org/archive/modules/writer/WARCWriterChainProcessor.java

@@ -1,12 +1,14 @@
 package org.archive.modules.writer;
 
+import java.io.InputStream;
 import java.io.IOException;
 import java.net.URI;
 import java.util.Arrays;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.apache.commons.io.IOUtils;
 import org.archive.io.warc.WARCRecordInfo;
 import org.archive.io.warc.WARCWriter;
 import org.archive.modules.CrawlURI;
@@ -159,6 +161,17 @@ protected void writeRecords(CrawlURI curi, WARCWriter writer) throws IOException
                 WARCRecordInfo record = recordBuilder.buildRecord(curi, concurrentTo);
                 if (record != null) {
                     writer.writeRecord(record);
+                    InputStream is = null;
+                    try {
+                        is = record.getContentStream();
+                        is.close();
+                    }
+                    catch (Exception e){
+                        logger.log(Level.WARNING, "problem closing Warc Record Content Stream " + e);
+                    }
+                    finally {
+                        IOUtils.closeQuietly(record.getContentStream()); //Closing one way or the other seems to leave some file handles open. Calling close() and using closeQuietly() handles both FileStreams and FileChannels 
+                    }
                     if (concurrentTo == null) {
                         concurrentTo = record.getRecordId();
                     }