Skip to content

Commit

Permalink
Fix embed thread quitting too early
Browse files Browse the repository at this point in the history
  • Loading branch information
zcbenz committed Mar 21, 2018
1 parent ddc6c0c commit 700eb46
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 15 deletions.
22 changes: 21 additions & 1 deletion src/bootstrap.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
// setImmediate and process.nextTick makes use of uv_check and uv_prepare to
// run the callbacks, however since we only run uv loop on requests, the
// callbacks wouldn't be called until something else activated the uv loop,
// which would delay the callbacks for arbitrary long time. So we should
// initiatively activate the uv loop once setImmediate and process.nextTick is
// called.
function wrapWithActivateUvLoop(func) {
return function() {
process.activateUvLoop()
return func.apply(this, arguments)
}
}

(function bootstrap(NativeModule) {
// Make async method work.
const timers = NativeModule.require('timers')
process.nextTick = wrapWithActivateUvLoop(process.nextTick)
global.setImmediate = wrapWithActivateUvLoop(timers.setImmediate)
global.setTimeout = wrapWithActivateUvLoop(timers.setTimeout)
global.setInterval = wrapWithActivateUvLoop(timers.setInterval)

// Turn our modules into built-in modules.
let exports = this
const exports = this
NativeModule._source.asar_archive = exports.asar_archive
NativeModule._source.asar_monkey_patch = exports.asar_monkey_patch
NativeModule._source.pickle = exports.pickle
Expand Down
35 changes: 33 additions & 2 deletions src/node_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "node/deps/uv/src/uv-common.h"
#include "node/src/env-inl.h"
#include "node/src/node.h"
#include "node/src/node_internals.h"

namespace yode {

Expand All @@ -30,12 +31,17 @@ NodeIntegration::~NodeIntegration() {
// Clear uv.
uv_sem_destroy(&embed_sem_);
uv_close(reinterpret_cast<uv_handle_t*>(&wakeup_handle_), nullptr);
uv_close(reinterpret_cast<uv_handle_t*>(&next_tick_handle_), nullptr);
}

void NodeIntegration::Init() {
// Handled used for waking up the uv loop in embed thread.
// Handle used for waking up the uv loop in embed thread.
// Note that we does not unref this handle to keep the active_handles > 0.
uv_async_init(uv_loop_, &wakeup_handle_, nullptr);
uv_unref(reinterpret_cast<uv_handle_t*>(&wakeup_handle_));

// Handle used for invoking CallNextTick.
uv_async_init(uv_loop_, &next_tick_handle_, &OnCallNextTick);
uv_unref(reinterpret_cast<uv_handle_t*>(&next_tick_handle_));

// Start worker that will interrupt main loop when having uv events.
uv_sem_init(&embed_sem_, 0);
Expand All @@ -62,6 +68,14 @@ void NodeIntegration::UvRunOnce() {
uv_sem_post(&embed_sem_);
}

void NodeIntegration::CallNextTick() {
uv_async_send(&next_tick_handle_);
}

void NodeIntegration::ReleaseHandleRef() {
uv_unref(reinterpret_cast<uv_handle_t*>(&wakeup_handle_));
}

void NodeIntegration::WakeupMainThread() {
PostTask([this] {
this->UvRunOnce();
Expand Down Expand Up @@ -102,4 +116,21 @@ void NodeIntegration::EmbedThreadRunner(void *arg) {
}
}

// static
void NodeIntegration::OnCallNextTick(uv_async_t* handle) {
// Get current env.
v8::Isolate* isolate = v8::Isolate::GetCurrent();
v8::HandleScope handle_scope(isolate);
node::Environment* env = node::Environment::GetCurrent(isolate);
CHECK(env);

// The InternalCallbackScope can handle everything for us.
v8::Context::Scope context_scope(env->context());
node::InternalCallbackScope scope(
env,
v8::Local<v8::Object>(),
{0, 0},
node::InternalCallbackScope::kAllowEmptyResource);
}

} // namespace yode
14 changes: 13 additions & 1 deletion src/node_integration.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ class NodeIntegration {
// Run the libuv loop for once.
void UvRunOnce();

// Handle the nextTick callbacks.
void CallNextTick();

// Release our ref to uv handle, so active handles count become 0.
void ReleaseHandleRef();

protected:
NodeIntegration();

Expand All @@ -45,14 +51,20 @@ class NodeIntegration {

private:
// Thread to poll uv events.
static void EmbedThreadRunner(void *arg);
static void EmbedThreadRunner(void* arg);

// Handle nextTick callbacks.
static void OnCallNextTick(uv_async_t* handle);

// Whether the libuv loop has ended.
bool embed_closed_;

// Handle to wake up uv's loop.
uv_async_t wakeup_handle_;

// Handle to call nextTick callbacks.
uv_async_t next_tick_handle_;

// Thread for polling events.
uv_thread_t embed_thread_;

Expand Down
18 changes: 14 additions & 4 deletions src/yode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ void Log(const v8::FunctionCallbackInfo<v8::Value>& args) {
}
}

// Force running uv loop.
void ActivateUvLoop(const v8::FunctionCallbackInfo<v8::Value>& args) {
g_node_integration->CallNextTick();
}

// Invoke our bootstrap script.
void Bootstrap(const v8::FunctionCallbackInfo<v8::Value>& args) {
node::Environment* env = node::Environment::GetCurrent(args);
Expand All @@ -61,10 +66,10 @@ bool InitWrapper(node::Environment* env) {
// Initialize GUI after Node gets initialized.
v8::HandleScope handle_scope(env->isolate());
Init(env);
// process.log = Log
// Native methods.
env->SetMethod(env->process_object(), "log", &Log);
// process.bootstrap = Bootstrap
env->SetMethod(env->process_object(), "bootstrap", &Bootstrap);
env->SetMethod(env->process_object(), "activateUvLoop", &ActivateUvLoop);
// versions = process.versions
v8::Local<v8::Value> versions = env->process_object()->Get(
env->context(), ToV8(env, "versions")).ToLocalChecked();
Expand All @@ -74,13 +79,18 @@ bool InitWrapper(node::Environment* env) {
return true;
}

// Run uv loop for once before entering GUI message loop.
bool RunLoopWrapper(node::Environment* env) {
// Run uv loop for once before entering GUI message loop.
if (g_first_runloop) {
g_node_integration->UvRunOnce();
g_first_runloop = false;
}
return RunLoop(env);
// Run GUI message loop.
RunLoop(env);
// No need to keep uv loop alive.
g_node_integration->ReleaseHandleRef();
// Enter uv loop to handle unfinished uv tasks.
return uv_run(env->event_loop(), UV_RUN_DEFAULT);
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion src/yode.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ int Start(int argc, char* argv[]);
void Init(node::Environment* env);

// Run the GUI message loop for once, implemented by different platforms.
bool RunLoop(node::Environment* env);
void RunLoop(node::Environment* env);

} // namespace yode

Expand Down
3 changes: 1 addition & 2 deletions src/yode_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ void Init(node::Environment* env) {
gtk_init(nullptr, nullptr);
}

bool RunLoop(node::Environment* env) {
void RunLoop(node::Environment* env) {
gtk_main(); // block until quit
return uv_run(env->event_loop(), UV_RUN_DEFAULT);
}

} // namespace yode
3 changes: 1 addition & 2 deletions src/yode_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ void Init(node::Environment* env) {
[NSApp setActivationPolicy:NSApplicationActivationPolicyRegular];
}

bool RunLoop(node::Environment* env) {
void RunLoop(node::Environment* env) {
[NSApp run]; // block until quit
return uv_run(env->event_loop(), UV_RUN_DEFAULT);
}

} // namespace yode
3 changes: 1 addition & 2 deletions src/yode_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ void Init(node::Environment* env) {
#endif
}

bool RunLoop(node::Environment* env) {
void RunLoop(node::Environment* env) {
MSG msg;
while (::GetMessage(&msg, NULL, 0, 0)) {
::TranslateMessage(&msg);
::DispatchMessage(&msg);
}
return uv_run(env->event_loop(), UV_RUN_DEFAULT);
}

} // namespace yode
1 change: 1 addition & 0 deletions yode.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
'.',
'node/deps/cares/include', # for ares.h
'node/deps/uv/include', # for uv.h
'node/src', # for node things
],
'defines': [
'NODE_WANT_INTERNALS=1',
Expand Down

0 comments on commit 700eb46

Please sign in to comment.