Skip to content

Commit

Permalink
http: emit destroy for reused AsyncWraps from C++
Browse files Browse the repository at this point in the history
Refs: #19859
  • Loading branch information
basti1302 committed Oct 5, 2018
1 parent 64e5de6 commit 3d41d4f
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 17 deletions.
9 changes: 1 addition & 8 deletions lib/_http_client.js
Expand Up @@ -36,11 +36,7 @@ const {
const { OutgoingMessage } = require('_http_outgoing');
const Agent = require('_http_agent');
const { Buffer } = require('buffer');
const {
defaultTriggerAsyncIdScope,
destroyHooksExist,
emitDestroy
} = require('internal/async_hooks');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { URL, urlToOptions, searchParamsSymbol } = require('internal/url');
const { outHeadersKey, ondrain } = require('internal/http');
const {
Expand Down Expand Up @@ -636,9 +632,6 @@ function tickOnSocket(req, socket) {
var parser = parsers.alloc();
req.socket = socket;
req.connection = socket;
if (destroyHooksExist() && parser[is_reused_symbol] && parser.getAsyncId()) {
emitDestroy(parser.getAsyncId());
}
parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
parser.socket = socket;
parser.outgoing = req;
Expand Down
5 changes: 0 additions & 5 deletions lib/_http_server.js
Expand Up @@ -40,8 +40,6 @@ const { OutgoingMessage } = require('_http_outgoing');
const { outHeadersKey, ondrain } = require('internal/http');
const {
defaultTriggerAsyncIdScope,
destroyHooksExist,
emitDestroy,
getOrSetAsyncId
} = require('internal/async_hooks');
const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol;
Expand Down Expand Up @@ -341,9 +339,6 @@ function connectionListenerInternal(server, socket) {
socket.on('timeout', socketOnTimeout);

var parser = parsers.alloc();
if (destroyHooksExist() && parser[is_reused_symbol] && parser.getAsyncId()) {
emitDestroy(parser.getAsyncId());
}
parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
parser.socket = socket;
socket.parser = parser;
Expand Down
16 changes: 14 additions & 2 deletions src/async_wrap.cc
Expand Up @@ -413,7 +413,10 @@ void AsyncWrap::AsyncReset(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
double execution_async_id =
args[0]->IsNumber() ? args[0].As<Number>()->Value() : -1;
wrap->AsyncReset(execution_async_id);
// This assumes that when asyncReset is called from JS, it is always about
// reusing an existing AsyncWrap instance and never to initialize a freshly
// created AsyncWrap instance.
wrap->AsyncReset(execution_async_id, false, true);
}


Expand Down Expand Up @@ -605,7 +608,16 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
// Generalized call for both the constructor and for handles that are pooled
// and reused over their lifetime. This way a new uid can be assigned when
// the resource is pulled out of the pool and put back into use.
void AsyncWrap::AsyncReset(double execution_async_id, bool silent) {
void AsyncWrap::AsyncReset(double execution_async_id,
bool silent,
bool reused) {
if (reused) {
// If this instance was in use before, we have already emitted an init with
// its previous async_id and need to emit a matching destroy for that
// before generating a new async_id.
EmitDestroy(env(), get_async_id());
}

async_id_ =
execution_async_id == -1 ? env()->new_async_id() : execution_async_id;
trigger_async_id_ = env()->get_default_trigger_async_id();
Expand Down
4 changes: 3 additions & 1 deletion src/async_wrap.h
Expand Up @@ -147,7 +147,9 @@ class AsyncWrap : public BaseObject {

inline double get_trigger_async_id() const;

void AsyncReset(double execution_async_id = -1, bool silent = false);
void AsyncReset(double execution_async_id = -1,
bool silent = false,
bool reused = false);

// Only call these within a valid HandleScope.
v8::MaybeLocal<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
Expand Down
2 changes: 1 addition & 1 deletion src/node_http_parser.cc
Expand Up @@ -479,7 +479,7 @@ class Parser : public AsyncWrap, public StreamListener {
// We must only call AsyncReset for the latter case, because AsyncReset has
// already been called via the constructor for the former case.
if (isReused) {
parser->AsyncReset();
parser->AsyncReset(-1, false, true);
}
parser->Init(type);
}
Expand Down

0 comments on commit 3d41d4f

Please sign in to comment.