FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: add late code injector by RaisinTen · Pull Request #37503 · nodejs/node · GitHub

/ node Public

lib: add late code injector - #37503

Closed
RaisinTen wants to merge 3 commits into
nodejs:masterfrom
RaisinTen:lib/add-late-code-injector
Closed

lib: add late code injector#37503
RaisinTen wants to merge 3 commits into
nodejs:masterfrom
RaisinTen:lib/add-late-code-injector

Conversation

Copy link
Copy Markdown
Member

Fixes: #37440

RaisinTen mentioned this pull request Feb 24, 2021
Comment thread lib/internal/bootstrap/pre_execution.js Outdated
Comment thread lib/internal/bootstrap/pre_execution.js Outdated
Trott added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 24, 2021

Trott commented Feb 24, 2021

Copy link
Copy Markdown
Member

Should this be documented? Does this need a test?

Co-authored-by: Gireesh Punathil <gpunathi@in.ibm.com>

Copy link
Copy Markdown
Member Author

@Trott yes, will surely add documentation + tests for this after setupLateCodeInjector works as expected.

RaisinTen added the wip Issues and PRs that are still a work in progress. label Feb 24, 2021
RaisinTen marked this pull request as draft February 24, 2021 15:22

Copy link
Copy Markdown
Member

In the current form, this should definitely be implemented in userland through NODE_OPTIONS=--require=... instead

jasnell commented Feb 24, 2021

Copy link
Copy Markdown
Member

Yeah, without more information about the specific set of use cases being targeted here, I have to agree with @addaleax

Copy link
Copy Markdown
Member

In the current form, this should definitely be implemented in userland through NODE_OPTIONS=--require=... instead

@addaleax - how does that work? with --require, the module should be present at startup right? my use case has node process predates the module. In theory, the module can be developed even after the production has started.

devsnek commented Feb 24, 2021

Copy link
Copy Markdown
Member

@gireeshpunathil at startup, load a small module loader script, instead of the main script:

'use strict';
process.on('SIGUSR1', () => {
  require(process.env.THE_MODULE);
  delete require.cache[require.resolve(process.env.THE_MODULE)];
});

Copy link
Copy Markdown
Member

@devsnek - makes sense, in the given premise of this PR ( there are certain preparation possible prior to the process launch). However, the original issue does not have such a premise. How do we run an arbitrary piece of code in a running process, where the process did not undergo any preparatory steps.

This PR is probably a middleground approach wherein the name of the module is set on the env , but probably that is coming from an implementation challenge of:

  • unable to export env data after startup
  • unable to pass data through signals

, not because we have that flexibility.

mscdex commented Feb 24, 2021

Copy link
Copy Markdown
Contributor

If you want to run arbitrary code after launch, why not start a tcp server in conjunction with repl and/or readline?

Copy link
Copy Markdown
Member

This PR is probably a middleground approach wherein the name of the module is set on the env

I wouldn't call this PR a midde ground, though, because it has the exact same set of requirements as what @devsnek and I would be suggesting. If you want a no-preparation-at-all scenario, then you'll need a different approach.

Copy link
Copy Markdown
Member

If you want to run arbitrary code after launch, why not start a tcp server in conjunction with repl and/or readline?

@mscdex - that needs the target process to be prepared for receiving an arbitrary code, right? Here is my requirement, restating:

Run an arbitrary piece of code in a running process, where the process did not undergo any preparatory steps.

Copy link
Copy Markdown
Member

If you want a no-preparation-at-all scenario, then you'll need a different approach.

@addaleax - yes agree. Please let @RaisinTen / me know if you have ideas on those lines.

mscdex commented Feb 25, 2021

Copy link
Copy Markdown
Contributor

@gireeshpunathil It sounds like what you're wanting is impossible then. Even with this PR it requires an environment variable on startup of the process?

Copy link
Copy Markdown
Member

Even with this PR it requires an environment variable on startup of the process?

@mscdex - agree, this PR assumes we are informed about a future attach.

Copy link
Copy Markdown
Member Author

Thanks for the reviews. Closing in favour of #37748.

RaisinTen closed this Mar 15, 2021
RaisinTen deleted the lib/add-late-code-injector branch March 15, 2021 12:18
indutny added a commit that referenced this pull request May 13, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-minor PRs that contain new features and should be released in the next minor version. wip Issues and PRs that are still a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

late code injection

7 participants


Back | FazBrowse Home | New Git URL