Refine subprocess execution

src/main/java/jp/ac/titech/c/se/stein/app/blob/Historage.java

@@ -29,7 +29,7 @@
 /**
  * A Historage generator using universal-ctags.
  * Splits source files into finer-grained modules (one file per language object)
- * based on ctags output. For Java-specific splitting via JDT, see {@link HistorageViaJDT}.
+ * based on ctags output. For Java-specific splitting via JDT, see {@link HistorageJdt}.
  *
  * <p>Requires the {@code ctags} command to be available on the system.</p>
  */

src/main/java/jp/ac/titech/c/se/stein/app/blob/HistorageViaJDT.java → src/main/java/jp/ac/titech/c/se/stein/app/blob/HistorageJdt.java

@@ -36,7 +36,7 @@
 @Slf4j
 @ToString
 @Command(name = "@historage-jdt", description = "Generate finer-grained Java modules via JDT")
-public class HistorageViaJDT implements BlobTranslator {
+public class HistorageJdt implements BlobTranslator {
     public static final NameFilter JAVA = new NameFilter(true, "*.java");
 
     public static final Gson GSON = new Gson();

src/main/java/jp/ac/titech/c/se/stein/app/blob/Tokenize.java

@@ -17,7 +17,7 @@
  * The inverse operation is {@link Untokenize}.
  *
  * <p>Tokenization is language-agnostic, splitting on whitespace, word characters, and symbols.
- * For Java-aware tokenization, see {@link TokenizeViaJDT}.</p>
+ * For Java-aware tokenization, see {@link TokenizeJdt}.</p>
  */
 @ToString
 @Command(name = "@tokenize", description = "Encode source files to linetoken format")

src/main/java/jp/ac/titech/c/se/stein/app/blob/TokenizeViaJDT.java → src/main/java/jp/ac/titech/c/se/stein/app/blob/TokenizeJdt.java

@@ -23,10 +23,10 @@
 @Slf4j
 @ToString
 @Command(name = "@tokenize-jdt", description = "Encode Java source files to linetoken format via JDT")
-public class TokenizeViaJDT implements BlobTranslator {
+public class TokenizeJdt implements BlobTranslator {
     @Override
     public AnyHotEntry rewriteBlobEntry(final BlobEntry entry, final Context c) {
-        if (!HistorageViaJDT.JAVA.accept(entry)) {
+        if (!HistorageJdt.JAVA.accept(entry)) {
             return entry;
         }
         final String text = SourceText.of(entry.getBlob()).getContent();

src/main/java/jp/ac/titech/c/se/stein/util/ProcessRunner.java

@@ -18,8 +18,12 @@ public static boolean isAvailable(String command) {
         try {
             final Process p = new ProcessBuilder(command, "--version")
                     .redirectErrorStream(true).start();
-            p.getInputStream().readAllBytes();
-            return p.waitFor() == 0;
+            try {
+                p.getInputStream().readAllBytes();
+                return p.waitFor() == 0;
+            } finally {
+                p.destroyForcibly();
+            }
         } catch (Exception e) {
             return false;
         }
@@ -44,22 +48,29 @@ public ProcessRunner(final String[] cmdline, final Context c) throws IOException
     }
 
     /**
-     * Runs the given command, writing {@code input} to its stdin.
+     * Runs the given command, writing {@code input} to its stdin in a separate thread
+     * to avoid deadlock when the command's stdout buffer fills up.
      */
     public ProcessRunner(final String[] cmdline, final byte[] input, final Context c) throws IOException {
         this.proc = new ProcessBuilder()
                 .command(cmdline)
                 .redirectError(ProcessBuilder.Redirect.INHERIT)
                 .start();
-        // FIXME: does not work if the target command blocks
-        try (final OutputStream out = proc.getOutputStream()) {
-            out.write(input);
-        }
+        final Thread writer = new Thread(() -> {
+            try (final OutputStream out = proc.getOutputStream()) {
+                out.write(input);
+            } catch (IOException e) {
+                log.warn("Failed to write to stdin: {} {}", e.getMessage(), c);
+            }
+        });
+        writer.setDaemon(true);
+        writer.start();
         this.c = c;
     }
 
     /**
      * Returns a reader for the process's stdout. The reader is closed when this runner is closed.
+     * Mutually exclusive with {@link #getResult()}.
      */
     public BufferedReader getResultReader() {
         reader = new BufferedReader(new InputStreamReader(proc.getInputStream()));
@@ -68,6 +79,7 @@ public BufferedReader getResultReader() {
 
     /**
      * Reads and returns the entire stdout as a byte array.
+     * Mutually exclusive with {@link #getResultReader()}.
      */
     public byte[] getResult() {
         try (final InputStream in = proc.getInputStream()) {
@@ -79,11 +91,17 @@ public byte[] getResult() {
 
     @Override
     public void close() throws IOException {
-        try (final BufferedReader err = new BufferedReader(new InputStreamReader(proc.getErrorStream()))) {
-            err.lines().forEach(line -> log.warn("stderr: {} {}", line, c));
-        }
         if (reader != null) {
             reader.close();
         }
+        try {
+            final int exitCode = proc.waitFor();
+            if (exitCode != 0) {
+                log.warn("Process exited with code {}: {}", exitCode, c);
+            }
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            log.warn("Interrupted while waiting for process: {}", c);
+        }
     }
 }

src/test/java/jp/ac/titech/c/se/stein/app/blob/HistorageViaJDTTest.java → src/test/java/jp/ac/titech/c/se/stein/app/blob/HistorageJdtTest.java

@@ -1,6 +1,6 @@
 package jp.ac.titech.c.se.stein.app.blob;
 
-import jp.ac.titech.c.se.stein.app.blob.HistorageViaJDT.Module;
+import jp.ac.titech.c.se.stein.app.blob.HistorageJdt.Module;
 import jp.ac.titech.c.se.stein.core.Context;
 import jp.ac.titech.c.se.stein.core.SourceText;
 import jp.ac.titech.c.se.stein.entry.AnyHotEntry;
@@ -23,18 +23,18 @@
 
 import static org.junit.jupiter.api.Assertions.*;
 
-public class HistorageViaJDTTest {
+public class HistorageJdtTest {
 
     static String sampleSource;
     static RepositoryAccess source, result;
 
     @BeforeAll
     static void setUp() throws IOException {
-        try (InputStream is = HistorageViaJDTTest.class.getResourceAsStream("/sample/Hello.java.v3")) {
+        try (InputStream is = HistorageJdtTest.class.getResourceAsStream("/sample/Hello.java.v3")) {
             sampleSource = new String(is.readAllBytes(), StandardCharsets.UTF_8);
         }
         source = TestRepo.createSample();
-        result = TestRepo.rewrite(source,new HistorageViaJDT());
+        result = TestRepo.rewrite(source,new HistorageJdt());
     }
 
     @AfterAll
@@ -44,10 +44,10 @@ static void tearDown() {
     }
 
     List<Module> generateModules() {
-        return generateModules(new HistorageViaJDT());
+        return generateModules(new HistorageJdt());
     }
 
-    List<Module> generateModules(HistorageViaJDT historage) {
+    List<Module> generateModules(HistorageJdt historage) {
         SourceText text = SourceText.ofNormalized(sampleSource.getBytes(StandardCharsets.UTF_8));
         return historage.new ModuleGenerator("Hello.java", text).generate();
     }
@@ -125,31 +125,31 @@ public void testModuleContent() {
 
     @Test
     public void testExcludeClasses() {
-        HistorageViaJDT historage = new HistorageViaJDT();
+        HistorageJdt historage = new HistorageJdt();
         historage.requiresClasses = false;
         assertTrue(generateModules(historage).stream()
                 .noneMatch(m -> m.getFilename().endsWith(".cjava")));
     }
 
     @Test
     public void testExcludeMethods() {
-        HistorageViaJDT historage = new HistorageViaJDT();
+        HistorageJdt historage = new HistorageJdt();
         historage.requiresMethods = false;
         assertTrue(generateModules(historage).stream()
                 .noneMatch(m -> m.getFilename().endsWith(".mjava")));
     }
 
     @Test
     public void testExcludeFields() {
-        HistorageViaJDT historage = new HistorageViaJDT();
+        HistorageJdt historage = new HistorageJdt();
         historage.requiresFields = false;
         assertTrue(generateModules(historage).stream()
                 .noneMatch(m -> m.getFilename().endsWith(".fjava")));
     }
 
     @Test
     public void testDigestParameters() {
-        HistorageViaJDT historage = new HistorageViaJDT();
+        HistorageJdt historage = new HistorageJdt();
         historage.digestParameters = true;
         List<Module> modules = generateModules(historage);
 
@@ -166,15 +166,15 @@ public void testDigestParameters() {
     @Test
     public void testNonJavaFilePassedThrough() {
         BlobEntry entry = HotEntry.ofBlob("README.md", "# Hello");
-        HistorageViaJDT historage = new HistorageViaJDT();
+        HistorageJdt historage = new HistorageJdt();
         AnyHotEntry result = historage.rewriteBlobEntry(entry, Context.init());
         assertEquals(1, result.size());
         assertSame(entry, result.stream().findFirst().orElseThrow());
     }
 
     @Test
     public void testRequiresOriginals() {
-        HistorageViaJDT historage = new HistorageViaJDT();
+        HistorageJdt historage = new HistorageJdt();
         historage.requiresOriginals = false;
         BlobEntry entry = HotEntry.ofBlob("Hello.java", sampleSource);
         AnyHotEntry result = historage.rewriteBlobEntry(entry, Context.init());

src/test/java/jp/ac/titech/c/se/stein/app/blob/TokenizeViaJDTTest.java → src/test/java/jp/ac/titech/c/se/stein/app/blob/TokenizeJdtTest.java

@@ -9,33 +9,33 @@
 import static jp.ac.titech.c.se.stein.app.blob.TokenizeTest.tokens;
 import static org.junit.jupiter.api.Assertions.*;
 
-public class TokenizeViaJDTTest {
+public class TokenizeJdtTest {
 
     @Test
     public void testEncode() {
-        assertEquals("", TokenizeViaJDT.encode(""));
+        assertEquals("", TokenizeJdt.encode(""));
 
         assertEquals(tokens("int", " ", "x", " ", "=", " ", "1", ";"),
-                TokenizeViaJDT.encode("int x = 1;"));
+                TokenizeJdt.encode("int x = 1;"));
     }
 
     @Test
     public void testEncodePreservesComments() {
         // JDT scanner includes trailing newline in line comment token
         assertEquals(tokens("// comment\r", "int", " ", "x", ";"),
-                TokenizeViaJDT.encode("// comment\nint x;"));
+                TokenizeJdt.encode("// comment\nint x;"));
     }
 
     @Test
     public void testEncodePreservesStringLiteral() {
         assertEquals(tokens("String", " ", "s", " ", "=", " ", "\"hello\"", ";"),
-                TokenizeViaJDT.encode("String s = \"hello\";"));
+                TokenizeJdt.encode("String s = \"hello\";"));
     }
 
     @Test
     public void testEncodeMultiline() {
         assertEquals(tokens("class", " ", "A", " ", "{", "\r", "}"),
-                TokenizeViaJDT.encode("class A {\n}"));
+                TokenizeJdt.encode("class A {\n}"));
     }
 
     @Test
@@ -47,6 +47,6 @@ public void testRoundTrip() throws IOException {
     }
 
     private void assertRoundTrip(String source) {
-        assertEquals(source, Untokenize.decode(TokenizeViaJDT.encode(source)));
+        assertEquals(source, Untokenize.decode(TokenizeJdt.encode(source)));
     }
 }

src/test/java/jp/ac/titech/c/se/stein/app/commit/NoteCommitTest.java

@@ -1,7 +1,7 @@
 package jp.ac.titech.c.se.stein.app.commit;
 
-import jp.ac.titech.c.se.stein.app.blob.HistorageViaJDT;
-import jp.ac.titech.c.se.stein.app.blob.TokenizeViaJDT;
+import jp.ac.titech.c.se.stein.app.blob.HistorageJdt;
+import jp.ac.titech.c.se.stein.app.blob.TokenizeJdt;
 import jp.ac.titech.c.se.stein.core.RepositoryAccess;
 import jp.ac.titech.c.se.stein.testing.TestRepo;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -50,7 +50,7 @@ public void testSingleTransform() {
         // Tokenize → NoteCommit: notes contain original commit IDs
         final List<RevCommit> sourceCommits = source.collectCommits("refs/heads/main");
 
-        try (RepositoryAccess tokenized = TestRepo.rewrite(source,new TokenizeViaJDT());
+        try (RepositoryAccess tokenized = TestRepo.rewrite(source,new TokenizeJdt());
              RepositoryAccess noted = TestRepo.rewrite(tokenized,new NoteCommit())) {
 
             final List<RevCommit> commits = noted.collectCommits("refs/heads/main");
@@ -70,8 +70,8 @@ public void testDoubleTransform() {
         // Historage → Tokenize → NoteCommit: notes should still trace back to original
         final List<RevCommit> sourceCommits = source.collectCommits("refs/heads/main");
 
-        try (RepositoryAccess step1 = TestRepo.rewrite(source,new HistorageViaJDT());
-             RepositoryAccess step2 = TestRepo.rewrite(step1,new TokenizeViaJDT());
+        try (RepositoryAccess step1 = TestRepo.rewrite(source,new HistorageJdt());
+             RepositoryAccess step2 = TestRepo.rewrite(step1,new TokenizeJdt());
              RepositoryAccess noted = TestRepo.rewrite(step2,new NoteCommit())) {
 
             final List<RevCommit> commits = noted.collectCommits("refs/heads/main");

src/test/java/jp/ac/titech/c/se/stein/core/cache/PersistentEntryCacheTest.java

@@ -5,7 +5,7 @@
 
 import jp.ac.titech.c.se.stein.Application;
 import jp.ac.titech.c.se.stein.app.Identity;
-import jp.ac.titech.c.se.stein.app.blob.HistorageViaJDT;
+import jp.ac.titech.c.se.stein.app.blob.HistorageJdt;
 import jp.ac.titech.c.se.stein.rewriter.BlobTranslator;
 import jp.ac.titech.c.se.stein.rewriter.RepositoryRewriter;
 import jp.ac.titech.c.se.stein.testing.TestRepo;
@@ -85,11 +85,11 @@ public void testCacheMatchesNonCachedResult() {
     @Test
     public void testCacheWithHistorage() {
         try (RepositoryAccess target = TestRepo.create(true)) {
-            rewriteWithCache(new HistorageViaJDT().toRewriter(), target.repo);
+            rewriteWithCache(new HistorageJdt().toRewriter(), target.repo);
             final List<RevCommit> firstRun = target.collectCommits("refs/heads/main");
             assertFalse(firstRun.isEmpty());
 
-            rewriteWithCache(new HistorageViaJDT().toRewriter(), target.repo);
+            rewriteWithCache(new HistorageJdt().toRewriter(), target.repo);
             final List<RevCommit> secondRun = target.collectCommits("refs/heads/main");
 
             assertEquals(firstRun.size(), secondRun.size());

src/test/java/jp/ac/titech/c/se/stein/rewriter/BlobTranslatorTest.java

@@ -1,7 +1,7 @@
 package jp.ac.titech.c.se.stein.rewriter;
 
-import jp.ac.titech.c.se.stein.app.blob.HistorageViaJDT;
-import jp.ac.titech.c.se.stein.app.blob.TokenizeViaJDT;
+import jp.ac.titech.c.se.stein.app.blob.HistorageJdt;
+import jp.ac.titech.c.se.stein.app.blob.TokenizeJdt;
 import jp.ac.titech.c.se.stein.core.Context;
 import jp.ac.titech.c.se.stein.entry.AnyHotEntry;
 import jp.ac.titech.c.se.stein.entry.Entry;
@@ -88,11 +88,11 @@ public void testSplitThenTransform() {
     @Test
     public void testFinerGit() throws IOException {
         try (RepositoryAccess source = TestRepo.createSample()) {
-            final BlobTranslator composite = BlobTranslator.composite(new HistorageViaJDT(), new TokenizeViaJDT());
+            final BlobTranslator composite = BlobTranslator.composite(new HistorageJdt(), new TokenizeJdt());
 
             try (RepositoryAccess compositeResult = TestRepo.rewrite(source, composite);
-                 RepositoryAccess step1 = TestRepo.rewrite(source, new HistorageViaJDT());
-                 RepositoryAccess sequentialResult = TestRepo.rewrite(step1, new TokenizeViaJDT())) {
+                 RepositoryAccess step1 = TestRepo.rewrite(source, new HistorageJdt());
+                 RepositoryAccess sequentialResult = TestRepo.rewrite(step1, new TokenizeJdt())) {
 
                 final RevCommit compositeHead = compositeResult.getHead("refs/heads/main");
                 final RevCommit sequentialHead = sequentialResult.getHead("refs/heads/main");

src/test/java/jp/ac/titech/c/se/stein/rewriter/CommitTranslatorTest.java

@@ -1,6 +1,6 @@
 package jp.ac.titech.c.se.stein.rewriter;
 
-import jp.ac.titech.c.se.stein.app.blob.TokenizeViaJDT;
+import jp.ac.titech.c.se.stein.app.blob.TokenizeJdt;
 import jp.ac.titech.c.se.stein.app.commit.NoteCommit;
 import jp.ac.titech.c.se.stein.core.Context;
 import jp.ac.titech.c.se.stein.core.RepositoryAccess;
@@ -125,11 +125,11 @@ public void testCompositeWithBlobOnRepo() throws IOException {
         final List<RevCommit> sourceCommits = source.collectCommits("refs/heads/main");
 
         final CommitTranslator composite = CommitTranslator.composite(
-                CommitTranslator.fromBlob(new TokenizeViaJDT()),
+                CommitTranslator.fromBlob(new TokenizeJdt()),
                 new NoteCommit());
 
         try (RepositoryAccess compositeResult = TestRepo.rewrite(source, composite);
-             RepositoryAccess step1 = TestRepo.rewrite(source, new TokenizeViaJDT());
+             RepositoryAccess step1 = TestRepo.rewrite(source, new TokenizeJdt());
              RepositoryAccess sequentialResult = TestRepo.rewrite(step1, new NoteCommit())) {
 
             final List<RevCommit> compositeCommits = compositeResult.collectCommits("refs/heads/main");

src/test/java/jp/ac/titech/c/se/stein/testing/MemoryProfile.java

@@ -2,7 +2,7 @@
 
 import jp.ac.titech.c.se.stein.Application;
 import jp.ac.titech.c.se.stein.app.Identity;
-import jp.ac.titech.c.se.stein.app.blob.HistorageViaJDT;
+import jp.ac.titech.c.se.stein.app.blob.HistorageJdt;
 import jp.ac.titech.c.se.stein.core.Context;
 import jp.ac.titech.c.se.stein.rewriter.RepositoryRewriter;
 import jp.ac.titech.c.se.stein.util.TemporaryFile;
@@ -42,7 +42,7 @@ public static void main(String[] args) throws Exception {
             final FileRepository targetRepo = createRepository(tmp.getPath().toFile());
 
             final RepositoryRewriter rewriter = switch (command) {
-                case "historage" -> new HistorageViaJDT().toRewriter();
+                case "historage" -> new HistorageJdt().toRewriter();
                 default -> new Identity();
             };
             rewriter.setConfig(new Application.Config());