Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Sync node create #108

Merged
merged 6 commits into from
Jan 25, 2017
Merged

Sync node create #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1f4d974
Update nodejs_simple_sample JSON Windows config
zfields Jan 24, 2017
1854de8
Update nodejs_simple_sample JSON Linux config
zfields Jan 24, 2017
401f2d1
Fix build_nodejs.cmd allowing to set env vars
zfields Jan 24, 2017
b6b3260
Serialize nodeJS module create
zfields Jan 23, 2017
b09cc9c
Update integration tests
zfields Jan 25, 2017
48304ec
Address code review comments/concerns
zfields Jan 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 21 additions & 30 deletions bindings/nodejs/inc/nodejs_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#ifndef NODEJS_COMMON_H
#define NODEJS_COMMON_H

#include <future>
#include <string>
#include <utility>

#include "v8.h"

Expand All @@ -18,7 +20,6 @@ typedef void(*PFNMODULE_START)(NODEJS_MODULE_HANDLE_DATA* handle_data);
enum class NodeModuleState
{
error,
initializing,
initialized
};

Expand All @@ -30,8 +31,7 @@ struct NODEJS_MODULE_HANDLE_DATA
on_module_start(nullptr),
module_id(0),
v8_isolate(nullptr),
module_state(NodeModuleState::error),
start_pending(false)
module_state(NodeModuleState::error)
{
}

Expand All @@ -47,8 +47,7 @@ struct NODEJS_MODULE_HANDLE_DATA
on_module_start(module_start),
module_id(0),
v8_isolate(nullptr),
module_state(NodeModuleState::error),
start_pending(false)
module_state(NodeModuleState::error)
{
}

Expand All @@ -61,7 +60,6 @@ struct NODEJS_MODULE_HANDLE_DATA
on_module_start = rhs.on_module_start;
module_id = rhs.module_id;
module_state = rhs.module_state;
start_pending = rhs.start_pending;


if (v8_isolate != nullptr && rhs.module_object.IsEmpty() == false)
Expand All @@ -80,7 +78,6 @@ struct NODEJS_MODULE_HANDLE_DATA
on_module_start = rhs.on_module_start;
module_id = rhs.module_id;
module_state = rhs.module_state;
start_pending = rhs.start_pending;


if (v8_isolate != nullptr && rhs.module_object.IsEmpty() == false)
Expand All @@ -98,7 +95,6 @@ struct NODEJS_MODULE_HANDLE_DATA
on_module_start = rhs.on_module_start;
this->module_id = module_id;
module_state = rhs.module_state;
start_pending = rhs.start_pending;

if (v8_isolate != nullptr && rhs.module_object.IsEmpty() == false)
{
Expand Down Expand Up @@ -128,28 +124,23 @@ struct NODEJS_MODULE_HANDLE_DATA
module_state = state;
}

bool GetStartPending()
{
nodejs_module::LockGuard<NODEJS_MODULE_HANDLE_DATA> lock_guard(*this);
return start_pending;
}

void SetStartPending(bool isPending)
{
nodejs_module::LockGuard<NODEJS_MODULE_HANDLE_DATA> lock_guard(*this);
start_pending = isPending;
}

BROKER_HANDLE broker;
std::string main_path;
std::string configuration_json;
v8::Isolate *v8_isolate;
v8::Persistent<v8::Object> module_object;
size_t module_id;
PFNMODULE_START on_module_start;
NodeModuleState module_state;
bool start_pending;
nodejs_module::Lock object_lock;
BROKER_HANDLE broker;
std::string main_path;
std::string configuration_json;
v8::Isolate *v8_isolate;
v8::Persistent<v8::Object> module_object;
size_t module_id;
PFNMODULE_START on_module_start;
NodeModuleState module_state;
nodejs_module::Lock object_lock;

/*
* WARNING: The promise follows the lifespan of the class, NOT
* the lifespan of the handle data. This means the
* std::future<NodeModuleState> associated with this
* class will NOT survive a copy or move operation.
*/
std::promise<NodeModuleState> create_complete;
};

#endif /*NODEJS_COMMON_H*/
68 changes: 44 additions & 24 deletions bindings/nodejs/src/nodejs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#include <cstdlib>
#include <chrono>
#include <cstdbool>
#include <cstdlib>
#include <cstddef>
#include <new>
#include <sstream>
#include <string>
#include <vector>

#include "azure_c_shared_utility/gballoc.h"

#ifdef WIN32
Expand All @@ -12,11 +18,6 @@
#include <unistd.h>
#endif

#include <string>
#include <new>
#include <sstream>
#include <vector>

#include "uv.h"
#include "v8.h"
#include "node.h"
Expand Down Expand Up @@ -61,10 +62,13 @@
" }" \
"})();"

static void NODEJS_Destroy(MODULE_HANDLE module);
static void on_module_start(NODEJS_MODULE_HANDLE_DATA* handle_data);
static bool validate_input(BROKER_HANDLE broker, const NODEJS_MODULE_CONFIG* module_config);
void call_start_on_module(NODEJS_MODULE_HANDLE_DATA* handle_data);

static const size_t NODE_LOAD_TIMEOUT_S = 10;

static MODULE_HANDLE NODEJS_Create(BROKER_HANDLE broker, const void* configuration)
{
MODULE_HANDLE result;
Expand Down Expand Up @@ -110,7 +114,31 @@ static MODULE_HANDLE NODEJS_Create(BROKER_HANDLE broker, const void* configurati
}
else
{
handle_data->SetModuleState(NodeModuleState::initializing);
// Wait for the module to become fully initialized
NodeModuleState module_state;
auto create_gate = handle_data->create_complete.get_future();
switch (create_gate.wait_for(std::chrono::seconds(NODE_LOAD_TIMEOUT_S)))
{
case std::future_status::ready:
module_state = create_gate.get();
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: break indentation should align with line 121. Here and on line 127.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed (overly helpful VS caused this).

case std::future_status::deferred:
case std::future_status::timeout:
default:
module_state = NodeModuleState::error;
break;
}
if (NodeModuleState::initialized != module_state)
{
LogError("Node process failed to start");
modules_manager->RemoveModule(handle_data->module_id);
NODEJS_Destroy(reinterpret_cast<MODULE_HANDLE>(handle_data));
handle_data = NULL;
}
else
{
handle_data->SetModuleState(module_state);
}

/*Codes_SRS_NODEJS_13_005: [ NodeJS_Create shall return a non-NULL MODULE_HANDLE when successful. ]*/
result = reinterpret_cast<MODULE_HANDLE>(handle_data);
Expand Down Expand Up @@ -817,7 +845,7 @@ static void on_module_start(NODEJS_MODULE_HANDLE_DATA* handle_data)
v8::Isolate* isolate = handle_data->v8_isolate = v8::Isolate::GetCurrent();
if (isolate == nullptr)
{
handle_data->SetModuleState(NodeModuleState::error);
handle_data->create_complete.set_value(NodeModuleState::error);
LogError("v8 isolate is nullptr");
}
else
Expand All @@ -827,7 +855,7 @@ static void on_module_start(NODEJS_MODULE_HANDLE_DATA* handle_data)
auto context = isolate->GetCurrentContext();
if (context.IsEmpty() == true)
{
handle_data->SetModuleState(NodeModuleState::error);
handle_data->create_complete.set_value(NodeModuleState::error);
LogError("No active v8 context found.");
}
else
Expand All @@ -839,7 +867,7 @@ static void on_module_start(NODEJS_MODULE_HANDLE_DATA* handle_data)
*/
if (create_gateway_host(isolate, context) == false)
{
handle_data->SetModuleState(NodeModuleState::error);
handle_data->create_complete.set_value(NodeModuleState::error);
LogError("Could not create gateway host in v8 global context");
}
else
Expand All @@ -861,32 +889,29 @@ static void on_module_start(NODEJS_MODULE_HANDLE_DATA* handle_data)
auto result = nodejs_module::NodeJSUtils::RunScript(isolate, context, script_str.str());
if (result.IsEmpty() == true)
{
handle_data->SetModuleState(NodeModuleState::error);
handle_data->create_complete.set_value(NodeModuleState::error);
LogError("registerModule script returned nullptr");
}
else
{
// this must be boolean and must evaluate to true
if (result->IsBoolean() == false)
{
handle_data->SetModuleState(NodeModuleState::error);
handle_data->create_complete.set_value(NodeModuleState::error);
LogError("Expected registerModule to return boolean but it did not.");
}
else
{
auto bool_result = result->ToBoolean();
if (bool_result->BooleanValue() == false)
{
handle_data->SetModuleState(NodeModuleState::error);
handle_data->create_complete.set_value(NodeModuleState::error);
LogError("Expected registerModule to return 'true' but it returned 'false'");
}
else
{
handle_data->SetModuleState(NodeModuleState::initialized);
if (handle_data->GetStartPending() == true)
{
call_start_on_module(handle_data);
}
handle_data->create_complete.set_value(NodeModuleState::initialized);
call_start_on_module(handle_data);
}
}
}
Expand Down Expand Up @@ -1227,11 +1252,7 @@ static void on_start_callback(
if (modules_manager->HasModule(module_id) == true)
{
auto& handle_data = modules_manager->GetModuleFromId(module_id);
if (handle_data.GetModuleState() == NodeModuleState::initializing)
{
handle_data.SetStartPending(true);
}
else if (handle_data.GetModuleState() == NodeModuleState::initialized)
if (handle_data.GetModuleState() == NodeModuleState::initialized)
{
if (handle_data.module_object.IsEmpty() == false)
{
Expand Down Expand Up @@ -1264,7 +1285,6 @@ static void on_start_callback(
{
LogError("Module does not have a JS object that needs to be destroyed.");
}
handle_data.SetStartPending(false);
}
else
{
Expand Down
Loading