fix(runtime): make kernel 'load' operation synchronous (#951)

The `jsii-runtime` process cannot perform async operations while waiting
for a (synchronous) callback to return (the response flow is otherwise
ambiguous). In order to allow loading of dependencies within a calllback
flow, the `load` kernel API needed to be made syncrhonous.

The reason why this is needed is because the host languages runtime
libraries may be required to load dependencies at the last minute, for
it may not be possible to determine what dependencies are required
before  a callback is executed.

Introduced a (unit) test to confirm the `jsii-runtime` is indeed able to
perform `load` operations within a callback context.

Author: RomainMuller

packages/jsii-kernel/lib/kernel.ts

@@ -56,16 +56,16 @@ export class Kernel {
     });
   }
 
-  public async load(req: api.LoadRequest): Promise<api.LoadResponse> {
+  public load(req: api.LoadRequest): api.LoadResponse {
     this._debug('load', req);
 
     if ('assembly' in req) {
       throw new Error('`assembly` field is deprecated for "load", use `name`, `version` and `tarball` instead');
     }
 
     if (!this.installDir) {
-      this.installDir = await fs.mkdtemp(path.join(os.tmpdir(), 'jsii-kernel-'));
-      await fs.mkdirp(path.join(this.installDir, 'node_modules'));
+      this.installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jsii-kernel-'));
+      fs.mkdirpSync(path.join(this.installDir, 'node_modules'));
       this._debug('creating jsii-kernel modules workdir:', this.installDir);
 
       process.on('exit', () => {
@@ -81,9 +81,9 @@ export class Kernel {
 
     // check if we already have such a module
     const packageDir = path.join(this.installDir, 'node_modules', pkgname);
-    if (await fs.pathExists(packageDir)) {
+    if (fs.pathExistsSync(packageDir)) {
       // module exists, verify version
-      const epkg = await fs.readJson(path.join(packageDir, 'package.json'));
+      const epkg = fs.readJsonSync(path.join(packageDir, 'package.json'));
       if (epkg.version !== pkgver) {
         throw new Error(`Multiple versions ${pkgver} and ${epkg.version} of the `
                 + `package '${pkgname}' cannot be loaded together since this is unsupported by `
@@ -101,19 +101,19 @@ export class Kernel {
     }
     // untar the archive to a staging directory, read the jsii spec from it
     // and then move it to the node_modules directory of the kernel.
-    const staging = await fs.mkdtemp(path.join(os.tmpdir(), 'jsii-kernel-install-staging-'));
+    const staging = fs.mkdtempSync(path.join(os.tmpdir(), 'jsii-kernel-install-staging-'));
     try {
-      await tar.extract({ strict: true, file: req.tarball, cwd: staging });
+      tar.extract({ strict: true, file: req.tarball, cwd: staging, sync: true });
 
       // read .jsii metadata from the root of the package
       const jsiiMetadataFile = path.join(staging, 'package', spec.SPEC_FILE_NAME);
-      if (!await fs.pathExists(jsiiMetadataFile)) {
+      if (!fs.pathExistsSync(jsiiMetadataFile)) {
         throw new Error(`Package tarball ${req.tarball} must have a file named ${spec.SPEC_FILE_NAME} at the root`);
       }
-      const assmSpec = await fs.readJson(jsiiMetadataFile) as spec.Assembly;
+      const assmSpec = fs.readJsonSync(jsiiMetadataFile) as spec.Assembly;
 
       // "install" to "node_modules" directory
-      await fs.move(path.join(staging, 'package'), packageDir);
+      fs.moveSync(path.join(staging, 'package'), packageDir);
 
       // load the module and capture it's closure
       const closure = this._execute(`require(String.raw\`${packageDir}\`)`, packageDir);
@@ -126,7 +126,7 @@ export class Kernel {
       };
     } finally {
       this._debug('removing staging directory:', staging);
-      await fs.remove(staging);
+      fs.removeSync(staging);
     }
 
   }

packages/jsii-kernel/test/kernel.test.ts

@@ -814,17 +814,18 @@ defineTest('fails: static methods - not static', (sandbox) => {
 });
 
 defineTest('loading a module twice idepotently succeeds', async (sandbox) => {
-  await sandbox.load({
+  sandbox.load({
     tarball: await preparePackage('jsii-calc', false),
     name: 'jsii-calc',
     version: calcVersion
   });
 });
 
-defineTest('fails if trying to load two different versions of the same module', async (sandbox) =>
-  expect(sandbox.load({ tarball: await preparePackage('jsii-calc', false), name: 'jsii-calc', version: '99.999.9' }))
-    .rejects.toThrow(/Multiple versions .+ and .+ of the package 'jsii-calc' cannot be loaded together/)
-);
+defineTest('fails if trying to load two different versions of the same module', async (sandbox) => {
+  const tarball = await preparePackage('jsii-calc', false);
+  return expect(() => sandbox.load({ tarball, name: 'jsii-calc', version: '99.999.9' }))
+    .toThrow(/Multiple versions .+ and .+ of the package 'jsii-calc' cannot be loaded together/)
+});
 
 defineTest('node.js standard library', async (sandbox) => {
   const objref = sandbox.create({ fqn: 'jsii-calc.NodeStandardLibrary' });
@@ -1221,9 +1222,9 @@ async function createCalculatorSandbox(name: string) {
 
   sandbox.traceEnabled = `${process.env.JSII_DEBUG}` === '1';
 
-  await sandbox.load({ tarball: await preparePackage('@scope/jsii-calc-base'), name: '@scope/jsii-calc-base', version: calcBaseVersion });
-  await sandbox.load({ tarball: await preparePackage('@scope/jsii-calc-lib'), name: '@scope/jsii-calc-lib', version: calcLibVersion });
-  await sandbox.load({ tarball: await preparePackage('jsii-calc'), name: 'jsii-calc', version: calcVersion });
+  sandbox.load({ tarball: await preparePackage('@scope/jsii-calc-base'), name: '@scope/jsii-calc-base', version: calcBaseVersion });
+  sandbox.load({ tarball: await preparePackage('@scope/jsii-calc-lib'), name: '@scope/jsii-calc-lib', version: calcLibVersion });
+  sandbox.load({ tarball: await preparePackage('jsii-calc'), name: 'jsii-calc', version: calcVersion });
   return sandbox;
 }
 

packages/jsii-runtime/.gitignore

@@ -8,3 +8,5 @@ webpack
 node_modules/
 .nyc_output/
 coverage/
+
+test/_tarballs/

packages/jsii-runtime/.npmignore

@@ -9,3 +9,5 @@ coverage
 .eslintrc.*
 tsconfig.json
 *.tsbuildinfo
+
+test/_tarballs/

packages/jsii-runtime/lib/host.ts

@@ -1,8 +1,10 @@
 import { api, Kernel } from 'jsii-kernel';
 import { Input, InputOutput } from './in-out';
+import { EventEmitter } from 'events';
 
 export class KernelHost {
   private readonly kernel = new Kernel(this.callbackHandler.bind(this));
+  private readonly eventEmitter = new EventEmitter();
 
   public constructor(private readonly inout: InputOutput, private readonly opts: { debug?: boolean, noStack?: boolean } = { }) {
     this.kernel.traceEnabled = opts.debug ? true : false;
@@ -11,6 +13,7 @@ export class KernelHost {
   public run() {
     const req = this.inout.read();
     if (!req) {
+      this.eventEmitter.emit('exit');
       return; // done
     }
 
@@ -21,6 +24,10 @@ export class KernelHost {
     });
   }
 
+  public on(event: 'exit', listener: () => void) {
+    this.eventEmitter.on(event, listener);
+  }
+
   private callbackHandler(callback: api.Callback) {
 
     // write a "callback" response, which is a special response that tells

packages/jsii-runtime/lib/index.ts

@@ -1,2 +1,2 @@
-// this module doesn't export any symbols
-
+export * from './host';
+export * from './in-out';

packages/jsii-runtime/package.json

@@ -28,8 +28,8 @@
     "build": "tsc --build && chmod +x bin/jsii-runtime && /bin/bash ./bundle.sh && npm run lint",
     "watch": "tsc --build -w",
     "lint": "eslint . --ext .js,.ts --ignore-path=.gitignore",
-    "test": "/bin/bash test/playback-test.sh && node test/stress-test.js",
-    "test:update": "UPDATE_DIFF=1 npm run test",
+    "test": "jest",
+    "test:update": "jest -u",
     "package": "package-js"
   },
   "dependencies": {
@@ -39,6 +39,7 @@
   "devDependencies": {
     "@scope/jsii-calc-base": "^0.20.1",
     "@scope/jsii-calc-lib": "^0.20.1",
+    "@types/jest": "^24.0.22",
     "@typescript-eslint/eslint-plugin": "^2.6.1",
     "@typescript-eslint/parser": "^2.6.1",
     "eslint": "^6.6.0",
@@ -51,5 +52,30 @@
     "wasm-loader": "^1.3.0",
     "webpack": "^4.41.2",
     "webpack-cli": "^3.3.10"
+  },
+  "jest": {
+    "collectCoverage": true,
+    "collectCoverageFrom": [
+      "**/bin/**/*.js",
+      "**/lib/**/*.js"
+    ],
+    "coverageReporters": [
+      "lcov",
+      "text"
+    ],
+    "coverageThreshold": {
+      "global": {
+        "branches": 45,
+        "statements": 57
+      }
+    },
+    "errorOnDeprecated": true,
+    "setupFilesAfterEnv": [
+      "jest-expect-message"
+    ],
+    "testEnvironment": "node",
+    "testMatch": [
+      "**/?(*.)+(spec|test).js"
+    ]
   }
 }

packages/jsii-runtime/test/__snapshots__/kernel-host.test.js.snap

@@ -0,0 +1,66 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`can load libraries from within a callback 1`] = `
+Object {
+  "ok": Object {
+    "assembly": "@scope/jsii-calc-base",
+    "types": "*redacted*",
+  },
+}
+`;
+
+exports[`can load libraries from within a callback 2`] = `
+Object {
+  "ok": Object {
+    "assembly": "@scope/jsii-calc-lib",
+    "types": "*redacted*",
+  },
+}
+`;
+
+exports[`can load libraries from within a callback 3`] = `
+Object {
+  "ok": Object {
+    "$jsii.byref": "Object@10000",
+    "$jsii.interfaces": Array [
+      "@scope/jsii-calc-lib.IFriendly",
+    ],
+  },
+}
+`;
+
+exports[`can load libraries from within a callback 4`] = `
+Object {
+  "callback": Object {
+    "cbid": "jsii::callback::20000",
+    "cookie": undefined,
+    "invoke": Object {
+      "args": Array [],
+      "method": "hello",
+      "objref": Object {
+        "$jsii.byref": "Object@10000",
+        "$jsii.interfaces": Array [
+          "@scope/jsii-calc-lib.IFriendly",
+        ],
+      },
+    },
+  },
+}
+`;
+
+exports[`can load libraries from within a callback 5`] = `
+Object {
+  "ok": Object {
+    "assembly": "jsii-calc",
+    "types": "*redacted*",
+  },
+}
+`;
+
+exports[`can load libraries from within a callback 6`] = `
+Object {
+  "ok": Object {
+    "result": "SUCCESS!",
+  },
+}
+`;

packages/jsii-runtime/test/kernel-host.test.ts

@@ -0,0 +1,87 @@
+import child = require('child_process');
+import fs = require('fs');
+import { api } from 'jsii-kernel';
+import spec = require('jsii-spec');
+import path = require('path');
+import { KernelHost, InputOutput, Input, Output } from '../lib';
+
+test('can load libraries from within a callback', () => {
+  const inout = new TestInputOutput(
+    [
+      { api: 'load', ...loadRequest('@scope/jsii-calc-base') },
+      { api: 'load', ...loadRequest('@scope/jsii-calc-lib') },
+      { api: 'create', fqn: 'Object', interfaces: ['@scope/jsii-calc-lib.IFriendly'], overrides: [{ method: 'hello' }] },
+      { api: 'invoke', objref: { [api.TOKEN_REF]: 'Object@10000' }, method: 'hello' },
+      { api: 'load', ...loadRequest('jsii-calc') },
+      { complete: { cbid: 'jsii::callback::20000', result: 'SUCCESS!' } },
+    ]
+  );
+  const host = new KernelHost(inout, { noStack: true, debug: false });
+  return new Promise<void>(ok => {
+    host.on('exit', () => ok(inout.expectCompleted()));
+    host.run();
+  });
+});
+
+class TestInputOutput extends InputOutput {
+  private readonly inputCommands: Input[];
+
+  public constructor(inputCommands: Input[], private readonly allowErrors = false) {
+    super();
+    this.inputCommands = inputCommands.reverse();
+  }
+
+  public read(): Input | undefined {
+    return this.inputCommands.pop();
+  }
+
+  public write(obj: Output): void {
+    if (!this.allowErrors) {
+      expect(obj).not.toHaveProperty('error');
+    }
+    if ('ok' in obj && 'assembly' in obj.ok) {
+      // Removing the type count as this is subject to change!
+      (obj.ok as any).types = '*redacted*';
+    }
+    expect(obj).toMatchSnapshot();
+  }
+
+  /**
+   * Validates that all inputs have been consumed, and all expected outputs have been checked.
+   */
+  public expectCompleted(): void {
+    expect(this.inputCommands).toEqual([]);
+  }
+}
+
+function loadRequest(library: string): api.LoadRequest {
+  const assembly = loadAssembly();
+  const tarball = path.join(__dirname, '_tarballs', library, `${assembly.fingerprint.replace('/', '_')}.tgz`);
+  if (!fs.existsSync(tarball)) {
+    packageLibrary(tarball);
+  }
+  return {
+    name: assembly.name,
+    version: assembly.version,
+    tarball,
+  };
+
+  function loadAssembly(): spec.Assembly {
+    const assemblyFile = path.resolve(require.resolve(`${library}/package.json`), '..', '.jsii');
+    return JSON.parse(fs.readFileSync(assemblyFile, { encoding: 'utf-8' }));
+  }
+
+  function packageLibrary(target: string): void {
+    const targetDir = path.dirname(target);
+    fs.mkdirSync(targetDir, { recursive: true });
+    const result = child.spawnSync('npm', ['pack', path.dirname(require.resolve(`${library}/package.json`))], { cwd: targetDir, stdio: ['inherit', 'pipe', 'pipe'] });
+    if (result.error) {
+      throw result.error;
+    }
+    if (result.status !== 0) {
+      console.error(result.stderr.toString('utf-8'));
+      throw new Error(`Unable to 'npm pack' ${library}: process ${result.signal != null ? `killed by ${result.signal}` : `exited with code ${result.status}`}`);
+    }
+    fs.renameSync(path.join(targetDir, result.stdout.toString('utf-8').trim()), target);
+  }
+}