Skip to content

Commit

Permalink
src: standardise context embedder indices
Browse files Browse the repository at this point in the history
PR-URL: nodejs#19135
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
  • Loading branch information
devsnek committed Mar 7, 2018
1 parent a6c14b2 commit c9b4de5
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 28 deletions.
11 changes: 7 additions & 4 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "uv.h"
#include "v8.h"
#include "node_perf_common.h"
#include "node_context_data.h"

#include <stddef.h>
#include <stdint.h>
Expand Down Expand Up @@ -283,7 +284,8 @@ inline void Environment::TickInfo::set_has_thrown(bool state) {

inline void Environment::AssignToContext(v8::Local<v8::Context> context,
const ContextInfo& info) {
context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
context->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kEnvironment, this);
#if HAVE_INSPECTOR
inspector_agent()->ContextCreated(context, info);
#endif // HAVE_INSPECTOR
Expand All @@ -295,7 +297,8 @@ inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {

inline Environment* Environment::GetCurrent(v8::Local<v8::Context> context) {
return static_cast<Environment*>(
context->GetAlignedPointerFromEmbedderData(kContextEmbedderDataIndex));
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kEnvironment));
}

inline Environment* Environment::GetCurrent(
Expand Down Expand Up @@ -368,8 +371,8 @@ inline Environment::~Environment() {
inspector_agent_.reset();
#endif

context()->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex,
nullptr);
context()->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kEnvironment, nullptr);

delete[] heap_statistics_buffer_;
delete[] heap_space_statistics_buffer_;
Expand Down
10 changes: 0 additions & 10 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,6 @@ struct PackageConfig {
};
} // namespace loader

// Pick an index that's hopefully out of the way when we're embedded inside
// another application. Performance-wise or memory-wise it doesn't matter:
// Context::SetAlignedPointerInEmbedderData() is backed by a FixedArray,
// worst case we pay a one-time penalty for resizing the array.
#ifndef NODE_CONTEXT_EMBEDDER_DATA_INDEX
#define NODE_CONTEXT_EMBEDDER_DATA_INDEX 32
#endif

// The number of items passed to push_values_to_array_function has diminishing
// returns around 8. This should be used at all call sites using said function.
#ifndef NODE_PUSH_VAL_TO_ARRAY_MAX
Expand Down Expand Up @@ -730,8 +722,6 @@ class Environment {
inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }

static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;

void AddPromiseHook(promise_hook_func fn, void* arg);
bool RemovePromiseHook(promise_hook_func fn, void* arg);
bool EmitNapiWarning();
Expand Down
25 changes: 25 additions & 0 deletions src/node_context_data.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#ifndef SRC_NODE_CONTEXT_DATA_H_
#define SRC_NODE_CONTEXT_DATA_H_

namespace node {

// Pick an index that's hopefully out of the way when we're embedded inside
// another application. Performance-wise or memory-wise it doesn't matter:
// Context::SetAlignedPointerInEmbedderData() is backed by a FixedArray,
// worst case we pay a one-time penalty for resizing the array.
#ifndef NODE_CONTEXT_EMBEDDER_DATA_INDEX
#define NODE_CONTEXT_EMBEDDER_DATA_INDEX 32
#endif

#ifndef NODE_CONTEXT_SANDBOX_OBJECT_INDEX
#define NODE_CONTEXT_SANDBOX_OBJECT_INDEX 33
#endif

enum ContextEmbedderIndex {
kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
};

} // namespace node

#endif // SRC_NODE_CONTEXT_DATA_H_
3 changes: 2 additions & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "node_watchdog.h"
#include "base_object-inl.h"
#include "node_contextify.h"
#include "node_context_data.h"

namespace node {
namespace contextify {
Expand Down Expand Up @@ -173,7 +174,7 @@ Local<Context> ContextifyContext::CreateV8Context(
// embedder data field. However, we cannot hold a reference to a v8::Context
// directly in an Object, we instead hold onto the new context's global
// object instead (which then has a reference to the context).
ctx->SetEmbedderData(kSandboxObjectIndex, sandbox_obj);
ctx->SetEmbedderData(ContextEmbedderIndex::kSandboxObject, sandbox_obj);
sandbox_obj->SetPrivate(env->context(),
env->contextify_global_private_symbol(),
ctx->Global());
Expand Down
7 changes: 2 additions & 5 deletions src/node_contextify.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
#include "node_internals.h"
#include "node_watchdog.h"
#include "base_object-inl.h"
#include "node_context_data.h"

namespace node {
namespace contextify {

class ContextifyContext {
protected:
// V8 reserves the first field in context objects for the debugger. We use the
// second field to hold a reference to the sandbox object.
enum { kSandboxObjectIndex = 1 };

Environment* const env_;
Persistent<v8::Context> context_;

Expand Down Expand Up @@ -45,7 +42,7 @@ class ContextifyContext {

inline v8::Local<v8::Object> sandbox() const {
return v8::Local<v8::Object>::Cast(
context()->GetEmbedderData(kSandboxObjectIndex));
context()->GetEmbedderData(ContextEmbedderIndex::kSandboxObject));
}

private:
Expand Down
7 changes: 4 additions & 3 deletions src/node_postmortem_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "util-inl.h"
#include "req_wrap.h"
#include "v8abbr.h"
#include "node_context_data.h"

#define NODEDBG_SYMBOL(Name) nodedbg_ ## Name

Expand Down Expand Up @@ -34,7 +35,7 @@
V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrap<uv_req_t>>::next_)

extern "C" {
int nodedbg_const_Environment__kContextEmbedderDataIndex__int;
int nodedbg_const_ContextEmbedderIndex__kEnvironment__int;
uintptr_t nodedbg_offset_ExternalString__data__uintptr_t;

#define V(Class, Member, Type, Accessor) \
Expand All @@ -46,8 +47,8 @@ uintptr_t nodedbg_offset_ExternalString__data__uintptr_t;
namespace node {

int GenDebugSymbols() {
nodedbg_const_Environment__kContextEmbedderDataIndex__int =
Environment::kContextEmbedderDataIndex;
nodedbg_const_ContextEmbedderIndex__kEnvironment__int =
ContextEmbedderIndex::kEnvironment;

nodedbg_offset_ExternalString__data__uintptr_t = NODE_OFF_EXTSTR_DATA;

Expand Down
10 changes: 5 additions & 5 deletions test/cctest/test_node_postmortem_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ extern uintptr_t
extern uintptr_t
nodedbg_offset_Environment__handle_wrap_queue___Environment_HandleWrapQueue;
extern int debug_symbols_generated;
extern int nodedbg_const_Environment__kContextEmbedderDataIndex__int;
extern int nodedbg_const_ContextEmbedderIndex__kEnvironment__int;
extern uintptr_t
nodedbg_offset_Environment_HandleWrapQueue__head___ListNode_HandleWrap;
extern uintptr_t
Expand Down Expand Up @@ -56,10 +56,10 @@ class TestReqWrap : public node::ReqWrap<uv_req_t> {
node::AsyncWrap::PROVIDER_TIMERWRAP) {}
};

TEST_F(DebugSymbolsTest, ContextEmbedderDataIndex) {
int kContextEmbedderDataIndex = node::Environment::kContextEmbedderDataIndex;
EXPECT_EQ(nodedbg_const_Environment__kContextEmbedderDataIndex__int,
kContextEmbedderDataIndex);
TEST_F(DebugSymbolsTest, ContextEmbedderEnvironmentIndex) {
int kEnvironmentIndex = node::ContextEmbedderIndex::kEnvironment;
EXPECT_EQ(nodedbg_const_ContextEmbedderIndex__kEnvironment__int,
kEnvironmentIndex);
}

TEST_F(DebugSymbolsTest, ExternalStringDataOffset) {
Expand Down

0 comments on commit c9b4de5

Please sign in to comment.