Skip to content

Commit

Permalink
Cleanup some things
Browse files Browse the repository at this point in the history
  • Loading branch information
Qard committed Jan 16, 2020
1 parent 58c109e commit d316772
Show file tree
Hide file tree
Showing 10 changed files with 14 additions and 30 deletions.
2 changes: 1 addition & 1 deletion lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function executionAsyncResource() {
const index = async_hook_fields[kStackLength] - 1;
if (index === -1) return topLevelResource;
const resource = execution_async_resources[index];
return resource || topLevelResource;
return resource;
}

// Used to fatally abort the process if a callback throws.
Expand Down
1 change: 0 additions & 1 deletion src/api/callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
object_(object),
skip_hooks_(flags & kSkipAsyncHooks),
skip_task_queues_(flags & kSkipTaskQueues) {
CHECK_IMPLIES(!(flags & kAllowEmptyResource), !object.IsEmpty());
CHECK_NOT_NULL(env);
env->PushAsyncCallbackScope();

Expand Down
12 changes: 5 additions & 7 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ inline AsyncHooks::AsyncHooks()
: async_ids_stack_(env()->isolate(), 16 * 2),
fields_(env()->isolate(), kFieldsCount),
async_id_fields_(env()->isolate(), kUidFieldsCount) {
v8::Isolate* isolate = env()->isolate();
v8::HandleScope handle_scope(isolate);

execution_async_resources_.Reset(isolate, v8::Array::New(isolate));
clear_async_id_stack();

// Always perform async_hooks checks, not just when async_hooks is enabled.
// TODO(AndreasMadsen): Consider removing this for LTS releases.
Expand Down Expand Up @@ -117,7 +114,7 @@ inline AliasedFloat64Array& AsyncHooks::async_ids_stack() {
}

inline v8::Local<v8::Array> AsyncHooks::execution_async_resources() {
return execution_async_resources_.Get(env()->isolate());
return PersistentToLocal::Strong(execution_async_resources_);
}

inline v8::Local<v8::String> AsyncHooks::provider_string(int idx) {
Expand Down Expand Up @@ -155,7 +152,7 @@ inline void AsyncHooks::push_async_context(double async_id,
async_id_fields_[kTriggerAsyncId] = trigger_async_id;

auto resources = execution_async_resources();
resources->Set(env()->context(), offset, resource).Check();
USE(resources->Set(env()->context(), offset, resource));
}

// Remember to keep this code aligned with popAsyncContext() in JS.
Expand Down Expand Up @@ -189,14 +186,15 @@ inline bool AsyncHooks::pop_async_context(double async_id) {
fields_[kStackLength] = offset;

auto resources = execution_async_resources();
resources->Delete(env()->context(), offset).FromJust();
USE(resources->Delete(env()->context(), offset));

return fields_[kStackLength] > 0;
}

// Keep in sync with clearAsyncIdStack in lib/internal/async_hooks.js.
inline void AsyncHooks::clear_async_id_stack() {
auto isolate = env()->isolate();
v8::HandleScope handle_scope(isolate);
execution_async_resources_.Reset(isolate, v8::Array::New(isolate));

async_id_fields_[kExecutionAsyncId] = 0;
Expand Down
3 changes: 1 addition & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#include <cstdint>
#include <functional>
#include <list>
#include <stack>
#include <unordered_map>
#include <unordered_set>
#include <vector>
Expand Down Expand Up @@ -684,7 +683,7 @@ class AsyncHooks : public MemoryRetainer {
inline Environment* env();

inline void push_async_context(double async_id, double trigger_async_id,
v8::Local<v8::Value> execution_async_resource_);
v8::Local<v8::Value> execution_async_resource_);
inline bool pop_async_context(double async_id);
inline void clear_async_id_stack(); // Used in fatal exceptions.

Expand Down
6 changes: 2 additions & 4 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,13 @@ class InternalCallbackScope {
public:
enum Flags {
kNoFlags = 0,
// Tell the constructor whether its `object` parameter may be empty or not.
kAllowEmptyResource = 1,
// Indicates whether 'before' and 'after' hooks should be skipped.
kSkipAsyncHooks = 2,
kSkipAsyncHooks = 1,
// Indicates whether nextTick and microtask queues should be skipped.
// This should only be used when there is no call into JS in this scope.
// (The HTTP parser also uses it for some weird backwards
// compatibility issues, but it shouldn't.)
kSkipTaskQueues = 4
kSkipTaskQueues = 2
};
InternalCallbackScope(Environment* env,
v8::Local<v8::Object> object,
Expand Down
3 changes: 1 addition & 2 deletions src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ int NodeMainInstance::Run() {
env.get(),
Object::New(isolate_),
{ 1, 0 },
InternalCallbackScope::kAllowEmptyResource |
InternalCallbackScope::kSkipAsyncHooks);
InternalCallbackScope::kSkipAsyncHooks);
LoadEnvironment(env.get());
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
if (env != nullptr) {
v8::HandleScope scope(isolate);
InternalCallbackScope cb_scope(env, Object::New(isolate), { 0, 0 },
InternalCallbackScope::kAllowEmptyResource);
InternalCallbackScope::kNoFlags);
task->Run();
} else {
task->Run();
Expand Down
3 changes: 1 addition & 2 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,7 @@ void Worker::Run() {
env_.get(),
Object::New(isolate_),
{ 1, 0 },
InternalCallbackScope::kAllowEmptyResource |
InternalCallbackScope::kSkipAsyncHooks);
InternalCallbackScope::kSkipAsyncHooks);

if (!env_->RunBootstrapping().IsEmpty()) {
CreateEnvMessagePort(env_.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const assert = require('assert');
const { executionAsyncResource, createHook } = require('async_hooks');
const { createServer, get } = require('http');
const sym = Symbol('cls');
const id = Symbol('id');

// Tests continuation local storage with the currentResource API
// through an async function
Expand All @@ -16,10 +15,7 @@ assert.ok(executionAsyncResource());
createHook({
init(asyncId, type, triggerAsyncId, resource) {
const cr = executionAsyncResource();
resource[id] = asyncId;
if (cr) {
resource[sym] = cr[sym];
}
resource[sym] = cr[sym];
}
}).enable();

Expand Down
6 changes: 1 addition & 5 deletions test/parallel/test-async-hooks-execution-async-resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const assert = require('assert');
const { executionAsyncResource, createHook } = require('async_hooks');
const { createServer, get } = require('http');
const sym = Symbol('cls');
const id = Symbol('id');

// Tests continuation local storage with the executionAsyncResource API

Expand All @@ -14,10 +13,7 @@ assert.ok(executionAsyncResource());
createHook({
init(asyncId, type, triggerAsyncId, resource) {
const cr = executionAsyncResource();
resource[id] = asyncId;
if (cr) {
resource[sym] = cr[sym];
}
resource[sym] = cr[sym];
}
}).enable();

Expand Down

0 comments on commit d316772

Please sign in to comment.