Embedding: static nodejs variables are not set to default

[Let0s]

• Version: v8.x (maybe any)
• Platform: MS Windows 7
• Subsystem: embedding

I use nodejs to run javascript code in my application. And I have found that after running script from code, script from file will not run. It is caused by unpredicted value of eval_string static variable, that was not set to nullptr at previous run, and bootstrap tries to run code instead of file. Also I think, some other static variables can interrupt running scripts too.

[addaleax]

As mentioned in nodejs/help#1367, this probably requires us to introduce a proper options parse/options management system.

[antsmartian]

@addaleax Not sure, what sort of help is needed here. But it would be great, if you can say what needs to be done (looks like its going to be a long list though) and will see if I can provide some help on it. Thanks!

[addaleax]

@antoaravinth Yes, it’s not going to be a short thing – this probably takes a long time to fully figure out, and it’s mostly C++ work.

I think the roadmap would look something like this:

• A first step would be categorizing Node CLI flags by their scope, i.e. per-Environment (e.g. --preserve-symlinks), per-Isolate (e.g. `), per-Thread, per-Process.
• Another first step would be working on an options parser for Node. Currently, we do this on a very ad-hoc basis, which doesn’t scale well and spreads out options handling over a large place.
• Separate parsing the options from them being accessed throughout Node.js
• Work out public APIs for parsing options objects and providing them for new Node.js Environment/Instances/etc.

I’m happy to help as well as I can, but yes, it’s going to be a bunch of work.

[antsmartian]

Thanks @addaleax for the response. Looks interesting, but unfortunately, I don't have good grasp on C++, so I can't help on that part. If anything that needs to be done from JS side, sure I'm ready to help here 👍

[addaleax]

Yeah, I’m sorry I only realized now that I should probably mentioned that this is a C++-heavy thing. :D

[ryansaam]

@addaleax I'm new to contributing but I would like to help out with the C++ issue. Is anyone working on this right now? I see @Let0s when I followed his reference link it seems Let0s is already coming up with a solution.

[addaleax]

@ryansaam There’s #22192, which you probably want to read for context. It should be quite a bit easier to do this properly now, but still requires some thought on the exact shape of the API.

I think that, eventually, we want a number of changes to the public API:

• Expose ProcessArgv() from node.cc
  • This is an (currently internal) API to change the global, per-process options object
  • This is tricky because ProcessArgv() also does some other things besides options parsing, like interpreting errors from there, etc.
    • Maybe the function should forward errors from the option parser, and errors of its own when appropriate
    • We might need to figure out some things that affect global state – e.g., what should ProcessArgv() do when it receives --v8-options? Should it still exit the process? Should it tell the caller to do so?
• Add variants of it for per-Isolate and per-Environment options
  • These would take an additional Environment* or IsolateData*` parameter, respectively
• For all three of these, add reset variants that reset all options to their default state
  • These should be relatively easy to implement, and address this issue, but doesn’t seem to make much sense without the others

(Also, ping @danbev in case you have any ideas here.)

[devasci]

Is there any work pending in this issue, i could work on this.