JSII runs into OOM issues when spawned through jsii-srcmak

[DanielMSchmidt]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

I was running into problems with jsii-srcmak on CI on terraform-cdk where it would go OOM. When debugging this we found that calling JSII directly on the same source ran with a third of the memory and no issues.

Here is the diff that solved the problem for me, I aligned the helper in jsii-pacmak:

diff --git a/node_modules/jsii-srcmak/lib/util.js b/node_modules/jsii-srcmak/lib/util.js
index f2cc7e4..69ca1a3 100644
--- a/node_modules/jsii-srcmak/lib/util.js
+++ b/node_modules/jsii-srcmak/lib/util.js
@@ -47,6 +47,8 @@ async function exec(moduleName, args = [], options = {}) {
         const opts = {
             ...options,
             stdio: ['inherit', 'pipe', 'pipe'],
+            shell: true,
+            env: { ...process.env }
         };
         const child = (0, child_process_1.spawn)(process.execPath, [moduleName, ...args], opts);
         const data = new Array();

Would you be open to a PR including this change?

[eladb]

Yes. Contributions are welcome

@RomainMuller any security concerns vis-à-vis shell: true?

[skorfmann]

•        shell: true,
  

•        env: { ...process.env }
  

@DanielMSchmidt do you know why shell and env made a difference?

[DanielMSchmidt]

I double checked today and found out that passing the env is not having an effect, just doing shell: true solves the issue. I was using shell here since the problem did not arise when running JSII directly from a shell, @leehambley said there might be underlying issues with signal handling.

[leehambley]

To clarify my assumption was that child processes started when it's node(danielmschmidt)->node(jsii)->node (I see numerous uses of exec in the jsii code, but I'm only looking as @DanielMSchmidt asked me if I had any ideas) that when jsii's parent program doesn't handle SIGCHLD or something, that spawned child processes maybe cannot exit cleanly and the memory is not released. I think that my advise to try with shell: true injecting a shell in between somwewhat confirmed that processes under JSII cannot exit/ or be disassociated properly

I'm not (really) a NodeJS developer, I see that it has some platform agnostic abstractions over the concept of SIGCHLD and so forth. Probably then, I suspect a shell is going something (handling SIGCHLD or similar) which Daniel's program isn't doing.

Pure speculation from my point, but maybe I'm going to work with Daniel offline and run this under strace -f -e trace=process .... to monitor what's happening with signals and exec...

[RomainMuller]

No security concern here... the first argument to exec is the fully qualified path to the node program... But it's not entirely clear why this has an effect on memory usage.

The only SIGCHLD related thing I could find was a node 0.x issue where SIGCHLD delivery can be VASTLY delayed in certain conditions, but it's not clear if this is the cast anymore or not...

Worth nothing that since we are spawning node children, we could use fork instead of spawn, too... I don't know if this would improve the memory situation here or not, but it would reduce the startup overhead...

[RomainMuller]

(Also, copying the env out made no difference because this is effectively the default value...)