diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7fc7f3cb9b..30a88404f4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -217,6 +217,7 @@ jobs: python: '3.6' # Test alternate .NETs - java: '8' + # Current release info can be found at https://dotnet.microsoft.com/download/dotnet/5.0 dotnet: '5.0.100-rc.2.20479.15' # Pre-release matching requires exact version for now node: '10' os: ubuntu-latest diff --git a/packages/@jsii/runtime/lib/program.ts b/packages/@jsii/runtime/lib/program.ts index bc7ab4f714..edc855739e 100644 --- a/packages/@jsii/runtime/lib/program.ts +++ b/packages/@jsii/runtime/lib/program.ts @@ -1,3 +1,5 @@ +import * as process from 'process'; + import * as packageInfo from '../package.json'; import { KernelHost } from './host'; import { InputOutput } from './in-out'; diff --git a/packages/@jsii/runtime/lib/sleep.ts b/packages/@jsii/runtime/lib/sleep.ts new file mode 100644 index 0000000000..002c23cd18 --- /dev/null +++ b/packages/@jsii/runtime/lib/sleep.ts @@ -0,0 +1,14 @@ +// We need a shared buffer array for the purpose of this exercise. +const array = new Int32Array(new SharedArrayBuffer(4)); + +/** + * Sleeps for a set amount of time, synchronously. + * + * @param time the amount of time to wait, in milliseconds. + */ +export function sleep(time: number): void { + // `Atomics.wait` will block for `time` milliseconds if `array[0]` still has + // value `0` (which it will, since we just initialized it to that). The return + // value is irrelevant for our business here. + Atomics.wait(array, 0, 0, time); +} diff --git a/packages/@jsii/runtime/lib/sync-stdio.ts b/packages/@jsii/runtime/lib/sync-stdio.ts index 0bfd15c5a7..e3a57e386a 100644 --- a/packages/@jsii/runtime/lib/sync-stdio.ts +++ b/packages/@jsii/runtime/lib/sync-stdio.ts @@ -1,14 +1,20 @@ import * as fs from 'fs'; -const STDIN_FD = 0; -const STDOUT_FD = 1; -const STDERR_FD = 2; -const INPUT_BUFFER_SIZE = 1024 * 1024; // not related to max line length +import { sleep } from './sleep'; -const EMPTY_BUFFER = Buffer.alloc(0); +// Note: the `process.std{in,out,err}.fd` is not part of the `@types/node` declarations, because +// those cannot model how those fields are guaranteed to be absent within the context of worker +// threads. The should be present here, but since we must resort to `as any`, we take the extra +// precaution of defaulting if those values are not present. + +const STDIN_FD: number = (process.stdin as any).fd ?? 0; +const STDOUT_FD: number = (process.stdout as any).fd ?? 1; +const STDERR_FD: number = (process.stderr as any).fd ?? 2; + +const INPUT_BUFFER_SIZE = 1_048_576; // 1MiB (aka: 1024 * 1024), not related to max line length export class SyncStdio { - private bufferedData = EMPTY_BUFFER; + private bufferedData = Buffer.alloc(0); public writeErrorLine(line: string) { this.writeBuffer(Buffer.from(`${line}\n`), STDERR_FD); @@ -21,28 +27,14 @@ export class SyncStdio { public readLine(): string | undefined { const buff = Buffer.alloc(INPUT_BUFFER_SIZE); while (!this.bufferedData.includes('\n', 0, 'utf-8')) { - try { - const read = fs.readSync(STDIN_FD, buff, 0, buff.length, null); + const read = readSync(STDIN_FD, buff, 0, buff.length); - if (read === 0) { - return undefined; - } - - const newData = buff.slice(0, read); - this.bufferedData = Buffer.concat([this.bufferedData, newData]); - } catch (e) { - // HACK: node may set O_NONBLOCK on it's STDIN depending on what kind of input it is made - // of (see https://github.com/nodejs/help/issues/2663). When STDIN has O_NONBLOCK, calls may - // result in EAGAIN. In such cases, the call should be retried until it succeeds. This kind - // of polling will result in horrible CPU thrashing, but there does not seem to be a way to - // force a O_SYNC access to STDIN in a reliable way within node. - // In order to make this stop we need to either stop depending on synchronous reads, or to - // provision our own communication channel that can reliably be synchronous. This work is - // "tracked" at https://github.com/aws/aws-cdk/issues/5187 - if (e.code !== 'EAGAIN') { - throw e; - } + if (read === 0) { + return undefined; } + + const newData = buff.slice(0, read); + this.bufferedData = Buffer.concat([this.bufferedData, newData]); } const newLinePos = this.bufferedData.indexOf('\n', 0, 'utf-8'); @@ -61,6 +53,64 @@ export class SyncStdio { if (e.code !== 'EAGAIN') { throw e; } + sleep(50 /*ms*/); + } + } + } +} + +/** + * A patched up implementation of `fs.readSync` that works around the quirks associated with + * synchronous I/O involving `STDIN`, `STDOUT` and `STDERR` on NodeJS, where this is all expected + * to be asynchronous and hence has some "interesting" behavior in certain particular cases. + * + * @param fd the file descriptor to read from (typically 0 / STDIN) + * @param buffer the buffer in which to place the read data + * @param offset the offset in the buffer at which to place the read data + * @param length the number of bytes to be read + * @param position where to begin reading from the file (defaults to the current read location) + * + * @returns the amount of bytes read, or `0` if EOF has been reached. + */ +function readSync( + fd: number, + buffer: Buffer, + offset: number, + length: number, + position: number | null = null, +): number { + // We are using a `while (true)` here to avoid recursively re-entering the function, in order to + // avoid thrashing memory with stack frames, when we are more likely to face `EAGAIN` on systems + // with low resources (near memory limit and/or high load). + // + // eslint-disable-next-line no-constant-condition + while (true) { + try { + return fs.readSync(fd, buffer, offset, length, position); + } catch (error) { + switch (error.code) { + // HACK: node may set O_NONBLOCK on it's STDIN depending on what kind of input it is made + // of (see https://github.com/nodejs/help/issues/2663). When STDIN has O_NONBLOCK, calls may + // result in EAGAIN. In such cases, the call should be retried until it succeeds. This kind + // of polling could result in horrible CPU thrashing: while we can sleep between two + // attempts, sleeping too much would slow everything to a crawl, and not enough would cause + // significant wasting of CPU cycles. + case 'EAGAIN': + // Keep trying until it no longer says EAGAIN. We'll be waiting a little before retrying + // in order to avoid thrashing the CPU like there is no tomorrow. This is not entirely + // ideal, but it has to do. + sleep(50 /*ms*/); + break; + + // HACK: in Windows, when STDIN (aka FD#0) is wired to a socket (as is the case when started + // as a subprocess with piped IO), `fs.readSync` will throw "Error: EOF: end of file, read" + // instead of returning 0 (which is what the documentation suggests it would do). This is + // currently believed to be a bug in `node`: https://github.com/nodejs/node/issues/35997 + case 'EOF': + return 0; + + default: + throw error; } } } diff --git a/packages/@jsii/runtime/test/playback.test.ts b/packages/@jsii/runtime/test/playback.test.ts index fed5cbba51..88b9f5a7d0 100644 --- a/packages/@jsii/runtime/test/playback.test.ts +++ b/packages/@jsii/runtime/test/playback.test.ts @@ -41,17 +41,19 @@ function createRecords(): string { [ ...process.execArgv, require.resolve('jest/bin/jest'), - '--coverage=false', + '--no-coverage', + '--runInBand', 'test/kernel.test.ts', ], { - env: { ...process.env, JSII_RECORD: records, JSII_NOSTACK: '1' }, - stdio: ['inherit', 'pipe', 'pipe'], cwd: path.resolve( require.resolve('@jsii/kernel/test/kernel.test.js'), '..', '..', ), + env: { ...process.env, JSII_RECORD: records, JSII_NOSTACK: '1' }, + stdio: ['inherit', 'pipe', 'pipe'], + timeout: 60_000, // 1 minute }, );