From 599fc52a2ca9fe2dad9e5d89f5b555bec41ad3c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Mon, 8 Jun 2020 13:47:02 +0200 Subject: [PATCH] fix(@jsii/runtime): "maximum call stack size exceeded" in SyncStdio.readLine When using node `>= 12.17`, `EAGAIN` errors consistently occur when trying to read from `stdin` when there is no available data. The retry mechanism for this was to recursively call back `SyncStdio.readLine`, which could evtnually lead to a "Maximum call stack size exceeded" error to occur (possibly hidden from the consumer, and later causing a "Stream closed" error). This changes how the retry mechanism works so it operates in a `while` loop instead of making a recursive call, completely avoiding to run into the growing stack issue. Fixes aws/aws-cdk#8288 Fixes aws/aws-cdk#5187 --- packages/@jsii/runtime/lib/sync-stdio.ts | 56 +++++++++++------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/packages/@jsii/runtime/lib/sync-stdio.ts b/packages/@jsii/runtime/lib/sync-stdio.ts index 9e7f5166a8..70753d3d02 100644 --- a/packages/@jsii/runtime/lib/sync-stdio.ts +++ b/packages/@jsii/runtime/lib/sync-stdio.ts @@ -18,45 +18,39 @@ export class SyncStdio { } public readLine(): string | undefined { - if (this.inputQueue.length > 0) { - return this.inputQueue.shift(); - } - - try { - const buff = Buffer.alloc(INPUT_BUFFER_SIZE); - const read = fs.readSync(STDIN_FD, buff, 0, buff.length, null); + const buff = Buffer.alloc(INPUT_BUFFER_SIZE); + while (this.inputQueue.length === 0) { + try { + 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; } - } - } 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; } } const next = this.inputQueue.shift(); - if (next == null) { - return this.readLine(); - } - return next; }