Pin non-primitive function parameters (#87)

* Trying to build a repro case

* Use Rehkitz’ repro

* Fix it

* Getting travis to run after .org to .com migration

* Travis Please!

* Updated the webhook, maybe now travis will update?

* Next day, maybe Travis will be happy now?

* Travis should work for sure now :)

Co-authored-by: Aaron Turner <aaron@aaronthedev.com>

lib/asbind-instance/bind-function.js

@@ -71,14 +71,25 @@ export function bindExportFunction(
         `Expected ${argumentConverterFunctions.length} arguments, got ${args.length}`
       );
     }
-    const newArgs = args.map((v, i) =>
-      argumentConverterFunctions[i](
+    // The conversion function of the _next_ parameter could potentially
+    // trigger GC and collect the previous parameter before we even invoke
+    // the actual function. That’s bad! We’ll pin all non-primitive parameters before invocation
+    // and unpin them after.
+    const pinnedArgs = [];
+    const newArgs = args.map((originalParameter, i) => {
+      const convertedParameter = argumentConverterFunctions[i](
         asbindInstance,
-        v,
+        originalParameter,
         exportedFunctionDescriptor.parameters[i]
-      )
-    );
+      );
+      if (convertedParameter !== originalParameter) {
+        asbindInstance.exports.__pin(convertedParameter);
+        pinnedArgs.push(convertedParameter);
+      }
+      return convertedParameter;
+    });
     const result = exportedFunction(...newArgs);
+    pinnedArgs.forEach(arg => asbindInstance.exports.__unpin(arg));
     return returnValueConverterFunction(
       asbindInstance,
       result,

test/test-runner.js

@@ -1,5 +1,6 @@
 const { promisify } = require("util");
 const fs = require("fs/promises");
+const { dirname, join } = require("path");
 
 const Express = require("express");
 const Mocha = require("mocha");
@@ -31,8 +32,17 @@ async function compileAllAsc() {
   const ascFiles = await glob("./tests/**/asc.ts");
   const transformFile = require.resolve("../dist/transform.cjs.js");
   for (const ascFile of ascFiles) {
+    const dir = dirname(ascFile);
+    let config = {
+      mangleCompilerParams() {}
+    };
+    try {
+      const configPath = require.resolve("./" + join(dir, "config.js"));
+      const m = require(configPath);
+      Object.assign(config, m);
+    } catch (e) {}
     console.log(`Compiling ${ascFile}...`);
-    await asc.main([
+    const params = [
       "--runtime",
       "stub",
       "--exportRuntime",
@@ -41,7 +51,9 @@ async function compileAllAsc() {
       "--binaryFile",
       ascFile.replace(/\.ts$/, ".wasm"),
       ascFile
-    ]);
+    ];
+    config.mangleCompilerParams(params);
+    await asc.main(params);
   }
 }
 

test/tests/pinning/asc.ts

@@ -0,0 +1,33 @@
+// The entry file of your WebAssembly module.
+
+declare function string_log(str: string): void;
+
+class MemoryTrash {
+  public t1: string;
+  public t2: string;
+  public t3: i32;
+
+  constructor() {
+    this.t1 = "trash1";
+    this.t2 = "trash1";
+    this.t3 = 42;
+  }
+}
+
+export function trash(amount: i32): void {
+  for (let i = 0; i < amount; i++) {
+    let t = new MemoryTrash();
+  }
+}
+
+export function string_parameter(
+  s1: string,
+  s2: string,
+  s3: string,
+  s4: string
+): void {
+  string_log(s1);
+  string_log(s2);
+  string_log(s3);
+  string_log(s4);
+}

test/tests/pinning/config.js

@@ -0,0 +1,8 @@
+exports.mangleCompilerParams = params => {
+  // Remove runtime parameter
+  const idx = params.indexOf("stub");
+  params.splice(idx - 1, 2);
+  // Add debug
+  params.unshift("--target", "debug");
+  console.log({ params });
+};

test/tests/pinning/test.js

@@ -0,0 +1,27 @@
+describe("as-bind", function() {
+  // Shoutout to @RehkitzDev for the repro
+  it("should not GC strings", function(done) {
+    let num_logs = 0;
+    function string_log(s) {
+      num_logs += 1;
+      assert(!/[^ABCD]/.test(s));
+      if (num_logs === 4) {
+        done();
+      }
+    }
+
+    (async () => {
+      const instance = await AsBind.instantiate(this.rawModule, {
+        asc: { string_log }
+      });
+
+      instance.exports.trash(10000);
+      let a = "A".repeat(30);
+      let b = "B".repeat(30);
+      let c = "C".repeat(60);
+      let d = "D".repeat(60);
+
+      instance.exports.string_parameter(a, b, c, d);
+    })();
+  });
+});