Skip to content

Commit

Permalink
fix(runtime): runtime crashes with EAGAIN trying to read from STDIN
Browse files Browse the repository at this point in the history
When using node >= 13.2, attempts to synchronously read from STDIN often
result in `EAGAIN` being raised. This is due to the STDIN file
descriptor being opened with `O_NONBLOCK`.

This introduces a temporary workaround that catches the EAGAIN exception
and re-attempts reading from STDIN "immediately". While this is not the
ideal solution, it can be done right away and appears to have minimal
impact on the overall performance of applications.

A longer term fix is to move away from synchronous IO operations, but
this is a significantly mroe involved fix and migth be a while before
this can be delivered. The long term solution is tracked in #1142.

Related to aws/aws-cdk#5187
  • Loading branch information
RomainMuller committed Dec 19, 2019
1 parent 242c55f commit 0a79d53
Showing 1 changed file with 24 additions and 12 deletions.
36 changes: 24 additions & 12 deletions packages/@jsii/runtime/lib/sync-stdio.ts
Expand Up @@ -22,21 +22,33 @@ export class SyncStdio {
return this.inputQueue.shift();
}

const buff = Buffer.alloc(INPUT_BUFFER_SIZE);
const read = fs.readSync(STDIN_FD, buff, 0, buff.length, null);
try {
const buff = Buffer.alloc(INPUT_BUFFER_SIZE);
const read = fs.readSync(STDIN_FD, buff, 0, buff.length, null);

if (read === 0) {
return undefined;
}
if (read === 0) {
return undefined;
}

const str = buff.slice(0, read).toString();
const str = buff.slice(0, read).toString();

for (const ch of str) {
if (ch === '\n') {
this.inputQueue.push(this.currentLine);
this.currentLine = '';
} else {
this.currentLine += ch;
for (const ch of str) {
if (ch === '\n') {
this.inputQueue.push(this.currentLine);
this.currentLine = '';
} else {
this.currentLine += ch;
}
}
} catch (e) {
// HACK: node opens STDIN with O_NONBLOCK, meaning attempts to synchrounously read from it may
// result in EAGAIN. In such cases, the call should be retried until it succeeds. This kind of
// polling may be terribly inefficient, and the "correct" way to address this is to stop
// relying on synchronous reads from STDIN. This is however a non-trivial endeavor, and the
// current state of things is very much broken in node >= 13.2, as can be see in
// https://github.com/aws/aws-cdk/issues/5187
if (e.code !== 'EAGAIN') {
throw e;
}
}

Expand Down

0 comments on commit 0a79d53

Please sign in to comment.