Fixed problem where temporary folders not being deleted causes errors on every compile. Closes #493

.gitignore

@@ -3,9 +3,11 @@
 */*.brk
 */*.prg
 */*.sym
+*/.tmpdirs
 */bin/
 */workspace.xml
 ./target/
 target/
 **/.DS_Store
 .project
+.tmpdirs

src/main/java/dk/camelot64/kickc/KickC.java

@@ -220,6 +220,9 @@ public class KickC implements Callable<Integer> {
 
       Program program = compiler.getProgram();
 
+      // Initialize tmp dir manager
+      TmpDirManager.init(program.getAsmFragmentBaseFolder());
+
       // Initialize the master ASM fragment synthesizer
       program.initAsmFragmentMasterSynthesizer(!optimizeNoFragmentCache);
 
@@ -486,6 +489,9 @@ public class KickC implements Callable<Integer> {
          }
       }
 
+      if(TmpDirManager.MANAGER!=null)
+         TmpDirManager.MANAGER.cleanup();
+
       return CommandLine.ExitCode.OK;
    }
 

src/main/java/dk/camelot64/kickc/TmpDirManager.java

@@ -0,0 +1,137 @@
+package dk.camelot64.kickc;
+
+import dk.camelot64.kickc.model.CompileError;
+
+import java.io.*;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Manages temporary folders with files.
+ * KickAssembler holds open handles to compiled ASM files. Therefore they cannot be deleted immediately after compilation.
+ * This manager
+ * - supports creation of new temporary folders
+ * - attempts deletion on exit
+ * - if deletion is not successful the folder paths are saved to a file. The folders in this file is attempted deleted on the next invocation.
+ */
+public class TmpDirManager {
+
+   /** Singleton manager. */
+   public static TmpDirManager MANAGER;
+
+   /**
+    * Initialize the singleton manager
+    *
+    * @param baseFolder The base folder where the TXT file containing dirs to delete will be loaded/saved from.
+    */
+   public static void init(Path baseFolder) {
+      MANAGER = new TmpDirManager(baseFolder);
+   }
+
+   /** The base folder where the TXT file containing dirs to delete will be loaded/saved from. */
+   private Path baseFolder;
+
+   /** Tmp folders that have been created and not deleted. */
+   private List<Path> tmpDirs;
+
+   private TmpDirManager(Path baseFolder) {
+      this.baseFolder = baseFolder;
+      this.tmpDirs = new ArrayList<>();
+   }
+
+   /**
+    * Creates a new temporary directory
+    *
+    * @return The new temporary folder
+    */
+   public Path newTmpDir() {
+      try {
+         Path tmpDir = Files.createTempDirectory("kickc");
+         this.tmpDirs.add(tmpDir);
+         return tmpDir;
+      } catch(IOException e) {
+         throw new CompileError("Error creating temporary directory. ", e);
+      }
+   }
+
+   /**
+    * Deletes all temporary folders - including any files in the folders.
+    * If deletion is not successful the absolute paths are saved to a txt-file to tried again later.
+    * This also loads the txt-file with previous failed deletions and retries them
+    */
+   public void cleanup() {
+      try {
+         // Attempt deletion of all temporary folders - including all files in the folders
+         List<Path> failedDirs = new ArrayList<>();
+         for(Path tmpDir : tmpDirs) {
+            boolean success = deleteTmpDir(tmpDir);
+            if(!success) {
+               failedDirs.add(tmpDir);
+               //System.out.println("Cannot delete temporary folder, postponing " + tmpDir);
+            } else {
+               //System.out.println("Successfully deleted temporary folder " + tmpDir);
+            }
+         }
+         // Read postponed file and delete any paths
+         File todoFile = baseFolder.resolve(".tmpdirs").toFile();
+         if(todoFile.exists()) {
+            FileReader todoFileReader = new FileReader(todoFile);
+            BufferedReader todoBufferedReader = new BufferedReader(todoFileReader);
+            String todoPathAbs = todoBufferedReader.readLine();
+            while(todoPathAbs != null) {
+               Path todoPath = new File(todoPathAbs).toPath();
+               boolean success = deleteTmpDir(todoPath);
+               if(!success) {
+                  failedDirs.add(todoPath);
+                  //System.out.println("Cannot delete postponed temporary folder - postponing again " + todoPathAbs);
+               } else {
+                  //System.out.println("Successfully deleted postponed temporary folder " + todoPathAbs);
+               }
+               todoPathAbs = todoBufferedReader.readLine();
+            }
+            todoBufferedReader.close();
+            todoFileReader.close();
+         }
+         // Delete the old postponed file
+         if(todoFile.exists()) {
+            if(!todoFile.delete())
+               System.err.println("Warning! Cannot delete .tmpdir  file " + todoFile.getAbsolutePath());
+            //System.out.println("Deleted old .tmpdir file " + todoFile.getAbsolutePath());
+         }
+         // Save any failed paths to new postponed file
+         if(failedDirs.size() > 0) {
+            PrintStream todoPrintStream = new PrintStream(todoFile);
+            for(Path failedDir : failedDirs) {
+               todoPrintStream.println(failedDir.toAbsolutePath());
+            }
+            todoPrintStream.close();
+            //System.out.println("Saved .tmpdir file with " + failedDirs.size() + " postponed temporary folders " + todoFile.getAbsolutePath());
+         }
+      } catch(IOException e) {
+         throw new CompileError("Error cleaning up temporary files", e);
+      }
+   }
+
+   private boolean deleteTmpDir(Path tmpDir) {
+      // Delete the temporary directory with folders
+      boolean success = true;
+      String[] entries = tmpDir.toFile().list();
+      if(entries != null)
+         for(String s : entries) {
+            File currentFile = new File(tmpDir.toFile(), s);
+            if(!currentFile.delete()) {
+               //System.err.println("Warning! Cannot delete temporary file " + currentFile.getAbsolutePath());
+               success = false;
+               break;
+            }
+         }
+      if(!tmpDir.toFile().delete()) {
+         //System.err.println("Warning! Cannot delete temporary folder " + tmpDir.toAbsolutePath());
+         success = false;
+      }
+      return success;
+   }
+
+}

src/main/java/dk/camelot64/kickc/passes/Pass5FixLongBranches.java

@@ -2,6 +2,7 @@ package dk.camelot64.kickc.passes;
 
 import dk.camelot64.cpufamily6502.CpuAddressingMode;
 import dk.camelot64.cpufamily6502.CpuOpcode;
+import dk.camelot64.kickc.TmpDirManager;
 import dk.camelot64.kickc.asm.*;
 import dk.camelot64.kickc.model.CompileError;
 import dk.camelot64.kickc.model.Program;
@@ -21,8 +22,6 @@ import java.util.regex.Pattern;
  */
 public class Pass5FixLongBranches extends Pass5AsmOptimization {
 
-   private Path tmpDir;
-
    public Pass5FixLongBranches(Program program) {
       super(program);
    }
@@ -54,35 +53,25 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization {
    private boolean step() {
       // Reindex ASM lines
       new Pass5ReindexAsmLines(getProgram()).optimize();
-
-      // Create a temporary directory for the ASM file
-      try {
-         tmpDir = Files.createTempDirectory("kickc");
-      } catch(IOException e) {
-         throw new CompileError("Error creating temp file.", e);
-      }
-
+      Path tmpDir = TmpDirManager.MANAGER.newTmpDir();
       // Generate the ASM file
       String outputFileName = getProgram().getPrimaryFileName();
       try {
          //getLog().append("ASM");
          //getLog().append(getProgram().getAsm().toString(false, true));
-
-         writeOutputFile(outputFileName, ".asm", getProgram().getAsm().toString(new AsmProgram.AsmPrintState(false), null));
-
+         writeOutputFile(tmpDir, outputFileName, ".asm", getProgram().getAsm().toString(new AsmProgram.AsmPrintState(false), null));
          // Copy Resource Files
          for(Path asmResourceFile : getProgram().getAsmResourceFiles()) {
-            File binFile = getTmpFile(asmResourceFile.getFileName().toString());
+            File binFile = getTmpFile(tmpDir, asmResourceFile.getFileName().toString());
             Files.copy(asmResourceFile, binFile.toPath());
          }
-
       } catch(IOException e) {
          throw new CompileError("Error writing ASM temp file.", e);
       }
 
       // Compile using KickAssembler - catch the output in a String
-      File asmFile = getTmpFile(outputFileName, ".asm");
-      File binaryFile = getTmpFile(outputFileName, "."+getProgram().getTargetPlatform().getOutFileExtension());
+      File asmFile = getTmpFile(tmpDir, outputFileName, ".asm");
+      File binaryFile = getTmpFile(tmpDir, outputFileName, "."+getProgram().getTargetPlatform().getOutFileExtension());
       ByteArrayOutputStream kickAssOut = new ByteArrayOutputStream();
       System.setOut(new PrintStream(kickAssOut));
       int asmRes = -1;
@@ -117,7 +106,6 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization {
                   // Found line number
                   //getLog().append("Found long branch line number "+contextLineIdx);
                   if(fixLongBranch(contextLineIdx - 1)) {
-                     removeTmpDir();
                      return true;
                   }
                }
@@ -125,25 +113,9 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization {
             }
          }
       }
-
-      removeTmpDir();
       return false;
    }
 
-   private void removeTmpDir() {
-      // Delete the temporary directory with folders
-      String[]entries = tmpDir.toFile().list();
-      for(String s: entries){
-         File currentFile = new File(tmpDir.toFile(),s);
-         if(!currentFile.delete()) {
-            System.err.println("Warning! Cannot delete temporary file "+currentFile.getAbsolutePath());
-         }
-      }
-      if(!tmpDir.toFile().delete()) {
-         System.err.println("Warning! Cannot delete temporary folder "+tmpDir.toAbsolutePath());
-      }
-   }
-
    /**
     * Fix a long branch detected at a specific ASM index
     *
@@ -202,9 +174,9 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization {
       }
    }
 
-   public File writeOutputFile(String fileName, String extension, String outputString) throws IOException {
+   private File writeOutputFile(Path tmpDir, String fileName, String extension, String outputString) throws IOException {
       // Write output file
-      File file = getTmpFile(fileName, extension);
+      File file = getTmpFile(tmpDir, fileName, extension);
       FileOutputStream outputStream = new FileOutputStream(file);
       OutputStreamWriter writer = new OutputStreamWriter(outputStream);
       writer.write(outputString);
@@ -214,12 +186,12 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization {
       return file;
    }
 
-   public File getTmpFile(String fileName, String extension) {
+   private static File getTmpFile(Path tmpDir, String fileName, String extension) {
       Path kcPath = FileSystems.getDefault().getPath(fileName);
       return new File(tmpDir.toFile(), kcPath.getFileName().toString() + extension);
    }
 
-   public File getTmpFile(String fileName) {
+   private static File getTmpFile(Path tmpDir, String fileName) {
       return new File(tmpDir.toFile(), fileName );
    }
 

src/test/java/dk/camelot64/kickc/test/TestKickAssRun.java

@@ -1,5 +1,6 @@
 package dk.camelot64.kickc.test;
 
+import dk.camelot64.kickc.TmpDirManager;
 import kickass.KickAssembler65CE02;
 import kickass.nonasm.c64.CharToPetsciiConverter;
 import org.junit.jupiter.api.Test;
@@ -19,16 +20,22 @@ public class TestKickAssRun {
     */
    @Test
    public void testKickAssRun() throws IOException, URISyntaxException {
+      TmpDirManager.init(new File("").toPath());
+
       ReferenceHelper asmHelper = new ReferenceHelperFolder("src/test/java/dk/camelot64/kickc/test/");
       URI asmUri = asmHelper.loadReferenceFile("kickasstest", ".asm");
       Path asmPath = Paths.get(asmUri);
-      File asmPrgFile = getTmpFile("kickasstest", ".prg");
+
+      Path tmpDir = TmpDirManager.MANAGER.newTmpDir();
+      File asmFile = getTmpFile(tmpDir, "kickasstest", ".asm");
+      File asmPrgFile = getTmpFile(tmpDir, "kickasstest", ".prg");
+      Files.copy(asmPath, asmFile.toPath());
       ByteArrayOutputStream kickAssOut = new ByteArrayOutputStream();
       System.setOut(new PrintStream(kickAssOut));
       try {
          CharToPetsciiConverter.setCurrentEncoding("screencode_mixed");
-         KickAssembler65CE02.main2(new String[]{asmPath.toAbsolutePath().toString(), "-o", asmPrgFile.getAbsolutePath()});
-      } catch (AssertionError e) {
+         KickAssembler65CE02.main2(new String[]{asmFile.getAbsolutePath(), "-o", asmPrgFile.getAbsolutePath()});
+      } catch(AssertionError e) {
          System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out)));
          String output = kickAssOut.toString();
          System.out.println(output);
@@ -38,11 +45,11 @@ public class TestKickAssRun {
       }
       String output = kickAssOut.toString();
       System.out.println(output);
+
+      TmpDirManager.MANAGER.cleanup();
    }
 
-
-   public File getTmpFile(String fileName, String extension) throws IOException {
-      Path tmpDir = Files.createTempDirectory("kickc");
+   public static File getTmpFile(Path tmpDir, String fileName, String extension) throws IOException {
       Path kcPath = FileSystems.getDefault().getPath(fileName);
       return new File(tmpDir.toFile(), kcPath.getFileName().toString() + extension);
    }
@@ -90,7 +97,7 @@ public class TestKickAssRun {
    private void printPetscii(String encoding, char ch, String sCh) {
       CharToPetsciiConverter.setCurrentEncoding(encoding);
       Byte petscii = CharToPetsciiConverter.convert(ch);
-      System.out.println(encoding+": "+sCh+" > "+(petscii==null?"null":(int)petscii));
+      System.out.println(encoding + ": " + sCh + " > " + (petscii == null ? "null" : (int) petscii));
    }
 
 

src/test/java/dk/camelot64/kickc/test/TestPrograms.java

@@ -3,6 +3,7 @@ package dk.camelot64.kickc.test;
 import dk.camelot64.kickc.CompileLog;
 import dk.camelot64.kickc.Compiler;
 import dk.camelot64.kickc.SourceLoader;
+import dk.camelot64.kickc.TmpDirManager;
 import dk.camelot64.kickc.asm.AsmProgram;
 import dk.camelot64.kickc.model.CompileError;
 import dk.camelot64.kickc.model.Program;
@@ -68,6 +69,7 @@ public class TestPrograms {
    public void testAtariXlMd5b() throws IOException, URISyntaxException {
       compileAndCompare("atarixl-md5b.c");
    }
+
    @Test
    public void testAtariXlMd5() throws IOException, URISyntaxException {
       compileAndCompare("atarixl-md5.c");
@@ -4822,10 +4824,13 @@ public class TestPrograms {
 
    @BeforeAll
    public static void setUp() {
+      TmpDirManager.init(new File("").toPath());
    }
 
    @AfterAll
    public static void tearDown() {
+      if(TmpDirManager.MANAGER != null)
+         TmpDirManager.MANAGER.cleanup();
       //AsmFragmentTemplateUsages.logUsages(log, false, false, false, false, false, false);
       //printGCStats();
    }