Merge pull request #2998 from asger-semmle/js/typescript-memory

Approved by erik-krogh

javascript/extractor/lib/typescript/src/main.ts

@@ -86,6 +86,8 @@ class State {
 }
 let state = new State();
 
+const reloadMemoryThresholdMb = getEnvironmentVariable("SEMMLE_TYPESCRIPT_MEMORY_THRESHOLD", Number, 1000);
+
 /**
  * Debugging method for finding cycles in the TypeScript AST. Should not be used in production.
  *
@@ -161,6 +163,7 @@ function extractFile(filename: string): string {
 function prepareNextFile() {
     if (state.pendingResponse != null) return;
     if (state.pendingFileIndex < state.pendingFiles.length) {
+        checkMemoryUsage();
         let nextFilename = state.pendingFiles[state.pendingFileIndex];
         state.pendingResponse = extractFile(nextFilename);
     }
@@ -529,26 +532,40 @@ function getEnvironmentVariable<T>(name: string, parse: (x: string) => T, defaul
     return value != null ? parse(value) : defaultValue;
 }
 
+/**
+ * Whether the memory usage was last observed to be above the threshold for restarting the TypeScript compiler.
+ *
+ * This is to prevent repeatedly restarting the compiler if the GC does not immediately bring us below the
+ * threshold again.
+ */
+let hasReloadedSinceExceedingThreshold = false;
+
+/**
+ * If memory usage has moved above a the threshold, reboot the TypeScript compiler instance.
+ *
+ * Make sure to call this only when stdout has been flushed.
+ */
+function checkMemoryUsage() {
+    let bytesUsed = process.memoryUsage().heapUsed;
+    let megabytesUsed = bytesUsed / 1000000;
+    if (!hasReloadedSinceExceedingThreshold && megabytesUsed > reloadMemoryThresholdMb && state.project != null) {
+        console.warn('Restarting TypeScript compiler due to memory usage');
+        state.project.reload();
+        hasReloadedSinceExceedingThreshold = true;
+    }
+    else if (hasReloadedSinceExceedingThreshold && megabytesUsed < reloadMemoryThresholdMb) {
+        hasReloadedSinceExceedingThreshold = false;
+    }
+}
+
 function runReadLineInterface() {
     reset();
-    let reloadMemoryThresholdMb = getEnvironmentVariable("SEMMLE_TYPESCRIPT_MEMORY_THRESHOLD", Number, 1000);
-    let isAboveReloadThreshold = false;
     let rl = readline.createInterface({ input: process.stdin, output: process.stdout });
     rl.on("line", (line: string) => {
         let req: Command = JSON.parse(line);
         switch (req.command) {
         case "parse":
             handleParseCommand(req);
-            // If memory usage has moved above the threshold, reboot the TypeScript compiler instance.
-            let bytesUsed = process.memoryUsage().heapUsed;
-            let megabytesUsed = bytesUsed / 1000000;
-            if (!isAboveReloadThreshold && megabytesUsed > reloadMemoryThresholdMb && state.project != null) {
-                console.warn('Restarting TypeScript compiler due to memory usage');
-                state.project.reload();
-                isAboveReloadThreshold = true;
-            } else if (isAboveReloadThreshold && megabytesUsed < reloadMemoryThresholdMb) {
-                isAboveReloadThreshold = false;
-            }
             break;
         case "open-project":
             handleOpenProjectCommand(req);

javascript/extractor/src/com/semmle/js/parser/TypeScriptParser.java

@@ -17,6 +17,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
@@ -37,7 +38,6 @@ import com.semmle.util.exception.Exceptions;
 import com.semmle.util.exception.InterruptedError;
 import com.semmle.util.exception.ResourceError;
 import com.semmle.util.exception.UserError;
-import com.semmle.util.io.WholeIO;
 import com.semmle.util.logging.LogbackUtils;
 import com.semmle.util.process.AbstractProcessBuilder;
 import com.semmle.util.process.Builder;
@@ -114,6 +114,18 @@ public class TypeScriptParser {
    */
   public static final String TYPESCRIPT_NODE_FLAGS = "SEMMLE_TYPESCRIPT_NODE_FLAGS";
 
+  /**
+   * Exit code for Node.js in case of a fatal error from V8. This exit code sometimes occurs
+   * when the process runs out of memory.
+   */
+  private static final int NODEJS_EXIT_CODE_FATAL_ERROR = 5;
+
+  /**
+   * Exit code for Node.js in case it exits due to <code>SIGABRT</code>. This exit code sometimes occurs
+   * when the process runs out of memory.
+   */
+  private static final int NODEJS_EXIT_CODE_SIG_ABORT = 128 + 6;
+
   /** The Node.js parser wrapper process, if it has been started already. */
   private Process parserWrapperProcess;
 
@@ -250,7 +262,7 @@ public class TypeScriptParser {
     int mainMemoryMb =
         typescriptRam != 0
             ? typescriptRam
-            : getMegabyteCountFromPrefixedEnv(TYPESCRIPT_RAM_SUFFIX, 1000);
+            : getMegabyteCountFromPrefixedEnv(TYPESCRIPT_RAM_SUFFIX, 2000);
     int reserveMemoryMb = getMegabyteCountFromPrefixedEnv(TYPESCRIPT_RAM_RESERVE_SUFFIX, 400);
 
     File parserWrapper = getParserWrapper();
@@ -318,15 +330,7 @@ public class TypeScriptParser {
     if (parserWrapperProcess == null) setupParserWrapper();
 
     if (!parserWrapperProcess.isAlive()) {
-      int exitCode = 0;
-      try {
-        exitCode = parserWrapperProcess.waitFor();
-      } catch (InterruptedException e) {
-        Exceptions.ignore(e, "This is for diagnostic purposes only.");
-      }
-      String err = new WholeIO().strictReadString(parserWrapperProcess.getErrorStream());
-      throw new CatastrophicError(
-          "TypeScript parser wrapper terminated with exit code " + exitCode + "; stderr: " + err);
+      throw getExceptionFromMalformedResponse(null, null);
     }
 
     String response = null;
@@ -335,13 +339,14 @@ public class TypeScriptParser {
       toParserWrapper.newLine();
       toParserWrapper.flush();
       response = fromParserWrapper.readLine();
-      if (response == null)
-        throw new CatastrophicError(
-            "Could not communicate with TypeScript parser wrapper "
-                + "(command: "
-                + parserWrapperCommand
-                + ").");
-      return new JsonParser().parse(response).getAsJsonObject();
+      if (response == null || response.isEmpty()) {
+        throw getExceptionFromMalformedResponse(response, null);
+      }
+      try {
+        return new JsonParser().parse(response).getAsJsonObject();
+      } catch (JsonParseException | IllegalStateException e) {
+        throw getExceptionFromMalformedResponse(response, e);
+      }
     } catch (IOException e) {
       throw new CatastrophicError(
           "Could not communicate with TypeScript parser wrapper "
@@ -349,17 +354,37 @@ public class TypeScriptParser {
               + parserWrapperCommand
               + ").",
           e);
-    } catch (JsonParseException | IllegalStateException e) {
-      throw new CatastrophicError(
-          "TypeScript parser wrapper sent unexpected response: "
-              + response
-              + " (command: "
-              + parserWrapperCommand
-              + ").",
-          e);
     }
   }
 
+  /**
+   * Creates an exception object describing the best known reason for the TypeScript parser wrapper
+   * failing to behave as expected.
+   *
+   * Note that the stderr stream is redirected to our stderr so a more descriptive error is likely
+   * to be found in the log, but we try to make the Java exception descriptive as well.
+   */
+  private RuntimeException getExceptionFromMalformedResponse(String response, Exception e) {
+    try {
+      Integer exitCode = null;
+      if (parserWrapperProcess.waitFor(1L, TimeUnit.SECONDS)) {
+        exitCode = parserWrapperProcess.waitFor();
+      }
+      if (exitCode != null && (exitCode == NODEJS_EXIT_CODE_FATAL_ERROR || exitCode == NODEJS_EXIT_CODE_SIG_ABORT)) {
+        return new ResourceError("The TypeScript parser wrapper crashed, possibly from running out of memory.", e);
+      }
+      if (exitCode != null) {
+        return new CatastrophicError("The TypeScript parser wrapper crashed with exit code " + exitCode);
+      }
+    } catch (InterruptedException e1) {
+      Exceptions.ignore(e, "This is for diagnostic purposes only.");
+    }
+    if (response == null) {
+      return new CatastrophicError("No response from TypeScript parser wrapper", e);
+    }
+    return new CatastrophicError("Unexpected response from TypeScript parser wrapper:\n" + response, e);
+  }
+
   /**
    * Returns the AST for a given source file.
    *