Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Embedding NodeJS to run many scripts #1031

Closed
Let0s opened this issue Dec 19, 2017 · 14 comments
Closed

Question: Embedding NodeJS to run many scripts #1031

Let0s opened this issue Dec 19, 2017 · 14 comments

Comments

@Let0s
Copy link

Let0s commented Dec 19, 2017

  • Node.js Version: various versions compiled from source code of "v8.x" and "master" branches
  • OS: MS Windows 7
  • Scope: Embedding

I want to embed NodeJS into an application. My idea is following:

  • make fork of node with my additional files
  • build modified library
  • use it in my application

but I want to run any script at any time. For access to node I wrote following code:

#include <node_version.h>
#include <v8.h>
#include <uv.h>

#include <env.h>
#include <env-inl.h>
#include <node_internals.h>
#include <node_platform.h>
#include <libplatform\libplatform.h>

namespace embed {
  const int DEFAULT_THREAD_POOL_SIZE = 4;
  bool initialized = false;
  node::NodePlatform* v8_platform;

  class Finalizer {
  public:
    ~Finalizer() {
      Fini();
    }
  }finalizer;

  //initialization v8 - once per process
  void Init() {
    if (!initialized) {
#if NODE_MAJOR_VERSION == 8
      v8_platform = new node::NodePlatform(DEFAULT_THREAD_POOL_SIZE, uv_default_loop(), nullptr);
#elif NODE_MAJOR_VERSION > 8
      v8_platform = new node::NodePlatform(DEFAULT_THREAD_POOL_SIZE, nullptr);
#endif
#if NODE_MAJOR_VERSION > 8
      node::tracing::TraceEventHelper::SetTracingController(
        new v8::TracingController());
#endif
      v8::V8::InitializePlatform(v8_platform);
      v8::V8::Initialize();

      initialized = true;
    }
  }

  //finalization v8 - once per process
  void Fini()
  {
    if (initialized) {
      v8::V8::Dispose();
      v8::V8::ShutdownPlatform();
      //v8_platform->Shutdown();
      delete v8_platform;
      v8_platform = nullptr;
      initialized = false;
    }
  }

  //running script
  void Run(int argc, const char * argv[])
  {
    node::ArrayBufferAllocator allocator;
    v8::Isolate::CreateParams params;
    params.array_buffer_allocator = &allocator;
    auto isolate = v8::Isolate::New(params);
    {
      v8::Locker locker(isolate);
      v8::Isolate::Scope isolate_scope(isolate);
      v8::HandleScope h_scope(isolate);

      v8::Local<v8::Context> context = v8::Context::New(isolate);
      v8::Context::Scope context_scope(context);
      node::IsolateData * isolate_data;
#if NODE_MAJOR_VERSION == 8
      isolate_data = node::CreateIsolateData(isolate, uv_default_loop());
#elif NODE_MAJOR_VERSION > 8
      isolate_data = node::CreateIsolateData(iso, uv_default_loop(), v8_platform);
#endif
      int exec_argc;
      const char ** exec_argv;
      node::Init(&argc, argv, &exec_argc, &exec_argv);
      auto env = node::CreateEnvironment(isolate_data,
        context,
        argc,
        argv,
        exec_argc,
        exec_argv);
      node::LoadEnvironment(env);
      node::FreeEnvironment(env);
      node::FreeIsolateData(isolate_data);
    }
    isolate->Dispose();
  }
}

All works fine (without errors) and I can call Run function many times when i want (of course, after calling Init function). But i don't like, that I must create v8::Isolate and node::Environment for every script. I tried to make one isolate for all scripts, but when I create second environment with the same isolate I get an error. I think, that one isolate and one environment for all scripts will be better way than code above. Is there a way to "reload" node::Environment with new argv? If no, is it possible to make this feature?

@Let0s Let0s changed the title Embedding NodeJS to run many scripts Question: Embedding NodeJS to run many scripts Dec 19, 2017
@bnoordhuis
Copy link
Member

but when I create second environment with the same isolate I get an error

What error?

@Let0s
Copy link
Author

Let0s commented Dec 19, 2017

image

related to this line. I think environment tries to add existing callback

@bnoordhuis
Copy link
Member

Sorry, been a while. I suspect it's caused by the built-in performance counters or ETW support. I filed nodejs/node#18074, help welcome.

@Let0s
Copy link
Author

Let0s commented Jan 11, 2018

It's not exactly what I wanted. The best way for me is create one isolate and one environment and just "reload" environment with new arguments for every script. But I think it's hard to realize my thoughts, isn't it?

@bnoordhuis
Copy link
Member

Correct.

@Let0s
Copy link
Author

Let0s commented Jan 12, 2018

I found one more bug realted to this theme.
As i described above, I use Run function for every script. But on node, builded from branches v9.x and master it runs one time. Second run doesn't work. In this line it loops and name parameter have value "natives". It caused by more than one call of node::Init function. On code from v8.x branch it works fine.
Simplest way to reproduce is:

  1. Get source code of node's v9.x or master branch
  2. Duplicate line with Init function call (link to line)
  3. Build node exe and run it.

@Let0s
Copy link
Author

Let0s commented Jan 12, 2018

If it can help:
OS: MS Windows 7 x64 (nodejs also x64)
My thoughts: when I call Init second time, it register modules again and make circular structure, where last node module's property nm_link links to first node module (or to another node module but not to nullptr). And nodejs try to find natives in infinite loop.

@gireeshpunathil
Copy link
Member

@Let0s - is this still outstanding?

@Let0s
Copy link
Author

Let0s commented Apr 6, 2018

@gireeshpunathil Yes, I think second part (with many calls Init function) is still important for embedders, because I think, that node::Init call is the only external way to pass arguments to nodejs (e.g path to script or its code).
If there is (or will be) another way, then this problem is not so important.

@gireeshpunathil
Copy link
Member

linking with nodejs/node#19377 in expectation that it is the most relevant one.

I guess the key question that is outstanding is:

Is there a way to "reload" node::Environment with new argv?

/cc @addaleax who has been leading the embedder's use cases.

@Let0s
Copy link
Author

Let0s commented Apr 6, 2018

Yes, this is the key question.

@gireeshpunathil
Copy link
Member

@Let0s - multi-thread based execution environment is being proposed through nodejs/node#20876 . You can find source that was refactored, as well as few APIs added in that PR - specifically in

  • src/env.cc
  • src/node.cc
  • src/node_platform.cc

that helps in re-entrance through a new Environment.

@Let0s
Copy link
Author

Let0s commented May 29, 2018

@gireeshpunathil Thank you for information.

@gireeshpunathil
Copy link
Member

closing this as answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants