fix(@jsii/runtime): "maximum call stack size exceeded" in SyncStdio.r…

…eadLine (#1717)

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
Fixes aws/aws-cdk#8397


---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

packages/@jsii/runtime/lib/sync-stdio.ts

@@ -18,45 +18,41 @@ 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 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;
         }
-      }
-    } 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;
   }