fix: pull in processing decompile phase from Forgeflower threading impl

This resolves a race condition when decompiling non-static inner classes
Vineflower · Aug 20, 2023 · e1caf59 · e1caf59
1 parent 1778698
commit e1caf59
Show file tree

Hide file tree

Showing 13 changed files with 208 additions and 108 deletions.
diff --git a/src/org/jetbrains/java/decompiler/main/ClassesProcessor.java b/src/org/jetbrains/java/decompiler/main/ClassesProcessor.java
@@ -401,7 +401,7 @@ else if (cl.superClass == null) { // neither interface nor super class defined
     return true;
   }
 
-  public void writeClass(StructClass cl, TextBuffer buffer) throws IOException {
+  public void processClass(StructClass cl) throws IOException {
     ClassNode root = mapRootClasses.get(cl.qualifiedName);
     if (root.type != ClassNode.Type.ROOT) {
       return;
@@ -410,45 +410,12 @@ public void writeClass(StructClass cl, TextBuffer buffer) throws IOException {
     boolean packageInfo = cl.isSynthetic() && "package-info".equals(root.simpleName);
     boolean moduleInfo = cl.hasModifier(CodeConstants.ACC_MODULE) && cl.hasAttribute(StructGeneralAttribute.ATTRIBUTE_MODULE);
 
-    DecompilerContext.getLogger().startReadingClass(cl.qualifiedName);
+    DecompilerContext.getLogger().startProcessingClass(cl.qualifiedName);
+    ImportCollector importCollector = new ImportCollector(root);
+    DecompilerContext.startClass(importCollector);
     try {
-      ImportCollector importCollector = new ImportCollector(root);
-      DecompilerContext.startClass(importCollector);
-
-      if (packageInfo) {
-        ClassWriter.packageInfoToJava(cl, buffer);
-
-        importCollector.writeImports(buffer, false);
-      }
-      else if (moduleInfo) {
-        TextBuffer moduleBuffer = new TextBuffer(AVERAGE_CLASS_SIZE);
-        ClassWriter.moduleInfoToJava(cl, moduleBuffer);
-
-        importCollector.writeImports(buffer, true);
-
-        buffer.append(moduleBuffer);
-      }
-      else {
-        try {
-          new LambdaProcessor().processClass(root);
-        } catch (Throwable t) {
-          DecompilerContext.getLogger().writeMessage("Class " + root.simpleName + " couldn't be written.",
-            IFernflowerLogger.Severity.WARN,
-            t);
-          buffer.append("// $VF: Couldn't be decompiled");
-          buffer.appendLineSeparator();
-          if (DecompilerContext.getOption(IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR)) {
-            List<String> lines = new ArrayList<>();
-            lines.addAll(ClassWriter.getErrorComment());
-            ClassWriter.collectErrorLines(t, lines);
-            for (String line : lines) {
-              buffer.append("//");
-              if (!line.isEmpty()) buffer.append(' ').append(line);
-              buffer.appendLineSeparator();
-            }
-          }
-          return;
-        }
+      if (!packageInfo && !moduleInfo) {
+        new LambdaProcessor().processClass(root);
 
         // add simple class names to implicit import
         addClassNameToImport(root, importCollector);
@@ -460,30 +427,42 @@ else if (moduleInfo) {
 
         if (spec == null) {
           // Java specific last minute processing
-          try {
             new NestedClassProcessor().processClass(root, root);
 
             new NestedMemberAccess().propagateMemberAccess(root);
-          } catch (Throwable t) {
-            DecompilerContext.getLogger().writeMessage("Class " + root.simpleName + " couldn't be written.",
-              IFernflowerLogger.Severity.WARN,
-              t);
-            buffer.append("// $VF: Couldn't be decompiled");
-            buffer.appendLineSeparator();
-            if (DecompilerContext.getOption(IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR)) {
-              List<String> lines = new ArrayList<>();
-              lines.addAll(ClassWriter.getErrorComment());
-              ClassWriter.collectErrorLines(t, lines);
-              for (String line : lines) {
-                buffer.append("//");
-                if (!line.isEmpty()) buffer.append(' ').append(line);
-                buffer.appendLineSeparator();
-              }
-            }
-            return;
-          }
         }
+      }
+    } finally {
+      DecompilerContext.getLogger().endProcessingClass();
+    }
+  }
 
+  public void writeClass(StructClass cl, TextBuffer buffer) throws IOException {
+    ClassNode root = mapRootClasses.get(cl.qualifiedName);
+    if (root.type != ClassNode.Type.ROOT) {
+      return;
+    }
+
+    boolean packageInfo = cl.isSynthetic() && "package-info".equals(root.simpleName);
+    boolean moduleInfo = cl.hasModifier(CodeConstants.ACC_MODULE) && cl.hasAttribute(StructGeneralAttribute.ATTRIBUTE_MODULE);
+
+    DecompilerContext.getLogger().startReadingClass(cl.qualifiedName);
+    try {
+      if (packageInfo) {
+        ClassWriter.packageInfoToJava(cl, buffer);
+
+        DecompilerContext.getImportCollector().writeImports(buffer, false);
+      }
+      else if (moduleInfo) {
+        TextBuffer moduleBuffer = new TextBuffer(AVERAGE_CLASS_SIZE);
+        ClassWriter.moduleInfoToJava(cl, moduleBuffer);
+
+        DecompilerContext.getImportCollector().writeImports(buffer, true);
+
+        buffer.append(moduleBuffer);
+      }
+      else {
+        LanguageSpec spec = PluginContext.getCurrentContext().getLanguageSpec(cl);
         TextBuffer classBuffer = new TextBuffer(AVERAGE_CLASS_SIZE);
         StatementWriter writer = spec != null ? spec.writer : new ClassWriter();
 
@@ -505,7 +484,7 @@ else if (moduleInfo) {
           }
         });
 
-        writer.writeClassHeader(cl, buffer, importCollector);
+        writer.writeClassHeader(cl, buffer, DecompilerContext.getImportCollector());
 
         int offsetLines = buffer.countLines();
 

diff --git a/src/org/jetbrains/java/decompiler/main/DecompilerContext.java b/src/org/jetbrains/java/decompiler/main/DecompilerContext.java
@@ -70,7 +70,11 @@ public static DecompilerContext getCurrentContext() {
   }
 
   public static void setCurrentContext(DecompilerContext context) {
-    currentContext.set(context);
+    if (context == null) {
+      currentContext.remove();
+    } else {
+      currentContext.set(context);
+    }
   }
 
   public static <T> void setProperty(Key<T> key, T value) {

diff --git a/src/org/jetbrains/java/decompiler/main/Fernflower.java b/src/org/jetbrains/java/decompiler/main/Fernflower.java
@@ -1,6 +1,7 @@
 // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package org.jetbrains.java.decompiler.main;
 
+import java.io.IOException;
 import org.jetbrains.java.decompiler.api.Plugin;
 import org.jetbrains.java.decompiler.main.ClassesProcessor.ClassNode;
 import org.jetbrains.java.decompiler.main.decompiler.OptionParser;
@@ -191,10 +192,15 @@ else if (converter != null) {
     }
   }
 
+  @Override
+  public void processClass(final StructClass cl) throws IOException {
+      classProcessor.processClass(cl); // unhandled exceptions handled later on
+  }
+
   @Override
   public String getClassContent(StructClass cl) {
+    TextBuffer buffer = new TextBuffer(ClassesProcessor.AVERAGE_CLASS_SIZE);
     try {
-      TextBuffer buffer = new TextBuffer(ClassesProcessor.AVERAGE_CLASS_SIZE);
       buffer.append(DecompilerContext.getProperty(IFernflowerPreferences.BANNER).toString());
       classProcessor.writeClass(cl, buffer);
 
@@ -206,7 +212,7 @@ public String getClassContent(StructClass cl) {
 
       String res = buffer.convertToStringAndAllowDataDiscard();
       if (res == null) {
-        return "$ FF: Unable to decompile class " + cl.qualifiedName;
+        return "$ VF: Unable to decompile class " + cl.qualifiedName;
       }
 
       return res;
@@ -220,7 +226,7 @@ public String getClassContent(StructClass cl) {
         lines.addAll(ClassWriter.getErrorComment());
         ClassWriter.collectErrorLines(t, lines);
         lines.add("*/");
-        return String.join("\n", lines);
+        return String.join(DecompilerContext.getNewLineSeparator(), lines);
       } else {
         return null;
       }

diff --git a/src/org/jetbrains/java/decompiler/main/decompiler/PrintStreamLogger.java b/src/org/jetbrains/java/decompiler/main/decompiler/PrintStreamLogger.java
@@ -32,6 +32,21 @@ public void writeMessage(String message, Severity severity, Throwable t) {
     }
   }
 
+  public void startProcessingClass(String className) {
+    if (accepts(Severity.INFO)) {
+      writeMessage("Preprocessing class " + className, Severity.INFO);
+      indent.get().incrementAndGet();
+    }
+  }
+
+  @Override
+  public void endProcessingClass() {
+    if (accepts(Severity.INFO)) {
+      indent.get().decrementAndGet();
+      writeMessage("... done", Severity.INFO);
+    }
+  }
+
   @Override
   public void startReadingClass(String className) {
     if (accepts(Severity.INFO)) {

diff --git a/src/org/jetbrains/java/decompiler/main/extern/IFernflowerLogger.java b/src/org/jetbrains/java/decompiler/main/extern/IFernflowerLogger.java
@@ -31,6 +31,10 @@ public void writeMessage(String message, Throwable t) {
     writeMessage(message, Severity.ERROR, t);
   }
 
+  public void startProcessingClass(String className) { }
+
+  public void endProcessingClass() { }
+
   public void startReadingClass(String className) { }
 
   public void endReadingClass() { }

diff --git a/src/org/jetbrains/java/decompiler/struct/ContextUnit.java b/src/org/jetbrains/java/decompiler/struct/ContextUnit.java
@@ -138,41 +138,62 @@ public void save(final Function<String, StructClass> loader) throws IOException
 
     //Whooo threads!
     final List<Future<?>> futures = new LinkedList<>();
-    final ExecutorService decompileExecutor = Executors.newFixedThreadPool(Integer.parseInt((String) DecompilerContext.getProperty(IFernflowerPreferences.THREADS)));
+    final int threads = Integer.parseInt((String) DecompilerContext.getProperty(IFernflowerPreferences.THREADS));
+    final ExecutorService workerExec = Executors.newFixedThreadPool(threads > 0 ? threads : Runtime.getRuntime().availableProcessors());
     final DecompilerContext rootContext = DecompilerContext.getCurrentContext();
-    final ClassContext[] toDump = new ClassContext[classEntries.size()];
+    final List<ClassContext> toDump = new ArrayList<>(classEntries.size());
 
-    // classes
+    // collect classes
     for (int i = 0; i < classEntries.size(); i++) {
       StructClass cl = loader.apply(classEntries.get(i));
       String entryName = decompiledData.getClassEntryName(cl, classEntries.get(i));
       if (entryName != null) {
-        final int finalI = i;
-        futures.add(decompileExecutor.submit(() -> {
-          setContext(rootContext);
-          String content = decompiledData.getClassContent(cl);
-          int[] mapping = null;
-          if (DecompilerContext.getOption(IFernflowerPreferences.BYTECODE_SOURCE_MAPPING)) {
-            mapping = DecompilerContext.getBytecodeSourceMapper().getOriginalLinesMapping();
-          }
-          toDump[finalI] = new ClassContext(cl.qualifiedName, entryName, content, mapping);
-        }));
+        toDump.add(new ClassContext(cl, entryName));
       }
     }
 
-    decompileExecutor.shutdown();
+    // pre-process
+    for (final ClassContext classCtx : toDump) {
+      futures.add(workerExec.submit(() -> {
+        setContext(rootContext);
+        classCtx.ctx = DecompilerContext.getCurrentContext();
+        try {
+          decompiledData.processClass(classCtx.cl);
+        } catch (final Throwable thr) {
+          classCtx.onError(thr);
+        } finally {
+          DecompilerContext.setCurrentContext(null);
+        }
+      }));
+    }
 
-    for (Future<?> future : futures) {
-      try {
-        future.get();
-      } catch (InterruptedException | ExecutionException e) {
-        throw new RuntimeException(e);
+    waitForAll(futures);
+    futures.clear();
+
+    // emit
+    for (final ClassContext classCtx : toDump) {
+      if (classCtx.pendingError != null) {
+        classCtx.classContent = "error"; // TODO
+        continue;
       }
+
+      futures.add(workerExec.submit(() -> {
+        DecompilerContext.setCurrentContext(classCtx.ctx);
+        classCtx.classContent = decompiledData.getClassContent(classCtx.cl);
+        if (DecompilerContext.getOption(IFernflowerPreferences.BYTECODE_SOURCE_MAPPING)) {
+          classCtx.mapping = DecompilerContext.getBytecodeSourceMapper().getOriginalLinesMapping();
+        }
+      }));
     }
 
+    waitForAll(futures);
+    futures.clear();
+    workerExec.shutdown();
+
+    // write to file
     for (final ClassContext cls : toDump) {
-      if (cls != null) {
-        sink.acceptClass(cls.qualifiedName, cls.entryName, cls.classContent, cls.mapping);
+      if (cls.classContent != null) {
+        sink.acceptClass(cls.cl.qualifiedName, cls.entryName, cls.classContent, cls.mapping);
       }
     }
 
@@ -194,6 +215,16 @@ public void setContext(DecompilerContext rootContext) {
     }
   }
 
+  private static void waitForAll(final List<Future<?>> futures) {
+    for (Future<?> future : futures) {
+      try {
+        future.get();
+      } catch (InterruptedException | ExecutionException e) {
+        throw new RuntimeException(e);
+      }
+    }
+  }
+
   public boolean isOwn() {
     return own;
   }
@@ -214,16 +245,25 @@ void close() throws Exception {
   }
 
   static final class ClassContext {
-    private final String qualifiedName;
+    private final StructClass cl;
+    DecompilerContext ctx;
     private final String entryName;
-    private final String classContent;
-    private final int /* @Nullable */[] mapping;
+    String classContent;
+    int /* @Nullable */[] mapping;
+    private Throwable pendingError;
 
-    ClassContext(final String qualifiedName, final String entryName, final String classContent, final int[] mapping) {
-      this.qualifiedName = qualifiedName;
+    ClassContext(final StructClass cl, final String entryName) {
+      this.cl = cl;
       this.entryName = entryName;
-      this.classContent = classContent;
-      this.mapping = mapping;
+    }
+
+    void onError(final Throwable thr) {
+      if (this.pendingError == null) {
+        this.pendingError = thr;
+        return;
+      }
+
+      this.pendingError.addSuppressed(thr);
     }
   }
 }
diff --git a/src/org/jetbrains/java/decompiler/struct/IDecompiledData.java b/src/org/jetbrains/java/decompiler/struct/IDecompiledData.java
@@ -1,9 +1,13 @@
 // Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package org.jetbrains.java.decompiler.struct;
 
+import java.io.IOException;
+
 public interface IDecompiledData {
 
   String getClassEntryName(StructClass cl, String entryname);
 
+  void processClass(StructClass cl) throws IOException;
+
   String getClassContent(StructClass cl);
 }
diff --git a/test/org/jetbrains/java/decompiler/SingleClassesTest.java b/test/org/jetbrains/java/decompiler/SingleClassesTest.java
@@ -634,6 +634,7 @@ private void registerDefault() {
 
     // TODO: when in different classes, these two decompile incorrectly to super(outer, s)
     register(JAVA_8, "TestInnerClassExtend");
+    register(JAVA_8, "TestInnerClassReference", "ext/SomeOuterClass", "ext/SomeOuterClass$SomeInner");
     register(JAVA_17, "TestInnerClassExtendJ17");
 
     // TODO: include scala stdlibs to avoid "import scala.runtime.ScalaRunTime."