Unexpected injection of NODE_OPTIONS environment variable

[jawnsy]

Terminals opened with code-server have NODE_OPTIONS="--max-old-space-size=2048" injected into the environment. This appears to come from this code: https://github.com/cdr/code-server/blob/d48a74c317f59d4a467ba004dcbe38abfefbf73f/src/node/wrapper.ts#L309-L319

I noticed this because our Dockerfile also injects the same setting as an environment variable, so it results in the same setting being set twice; we end up with: NODE_OPTIONS=--max-old-space-size=2048 --max-old-space-size=8192

I would propose that we remove the environment variable injection, as I don't think that's expected behavior from a users' perspective. If users want to control the environment of terminals opened with Code-Server (or VSCode), in a manner different from their shell settings (e.g. .bash_profile, .bashrc), they can always modify their workspace settings:

  // Object with environment variables that will be added to the VS Code process to be used by the terminal on Linux. Set to `null` to delete the environment variable.
  "terminal.integrated.env.linux": {},

  // Object with environment variables that will be added to the VS Code process to be used by the terminal on macOS. Set to `null` to delete the environment variable.
  "terminal.integrated.env.osx": {},

  // Object with environment variables that will be added to the VS Code process to be used by the terminal on Windows. Set to `null` to delete the environment variable.
  "terminal.integrated.env.windows": {},

If we just need the setting for code-server itself, then we might be able to use the --max-old-space-size=SIZE command-line flag, which would run our child process with the desired max old space size setting, but not propagate that option to VSCode's children (instead just using the NODE_OPTIONS we were provided).

cc @vapurrmaid

[greyscaled]

@jawnsy Great find, I had no idea code-server was performing this injection. I agree that's not expected at all. @code-asher do you happen to have any additional context on this behavior?

[code-asher]

I think this started as an attempt to support the max-memory flag of VS Code but we had that option fall back to 2048 instead of just not setting it. At some point the flag was removed and we were left with just the default.

I almost removed it but I was worried that the lower default limit might cause existing users to suddenly run into memory limit issues. But it does seem like a weird thing to do so I'm down to remove it.

For what it's worth Node will take the later flag when specified twice, at least according to the last time I tested.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no activity occurs in the next 5 days.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no activity occurs in the next 5 days.