From 3b58459472a473da80e13182a22bd153056b3939 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 4 Apr 2017 13:43:49 -0700 Subject: [PATCH 1/2] Fix call destruction bug --- ext/call.cc | 10 +++++++--- ext/call.h | 5 ++++- ext/channel.cc | 2 +- ext/server.cc | 4 ++-- ext/server_uv.cc | 3 ++- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index 244546d3..62f0130d 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -466,8 +466,10 @@ class ServerCloseResponseOp : public Op { int cancelled; }; -tag::tag(Callback *callback, OpVec *ops, Call *call) : +tag::tag(Callback *callback, OpVec *ops, Call *call, Local call_value) : callback(callback), ops(ops), call(call){ + HandleScope scope; + call_persist.Reset(call_value); } tag::~tag() { @@ -521,6 +523,7 @@ Call::Call(grpc_call *call) : wrapped_call(call), Call::~Call() { if (wrapped_call != NULL) { grpc_call_destroy(wrapped_call); + wrapped_call = NULL; } } @@ -567,7 +570,8 @@ void Call::CompleteBatch(bool is_final_op) { this->has_final_op_completed = true; } this->pending_batches--; - if (this->has_final_op_completed && this->pending_batches == 0) { + if (this->has_final_op_completed && this->pending_batches == 0 && + this->wrapped_call != NULL) { grpc_call_destroy(this->wrapped_call); this->wrapped_call = NULL; } @@ -721,7 +725,7 @@ NAN_METHOD(Call::StartBatch) { Callback *callback = new Callback(callback_func); grpc_call_error error = grpc_call_start_batch( call->wrapped_call, &ops[0], nops, new struct tag( - callback, op_vector.release(), call), NULL); + callback, op_vector.release(), call, info.This()), NULL); if (error != GRPC_CALL_OK) { return Nan::ThrowError(nanErrorWithCode("startBatch failed", error)); } diff --git a/ext/call.h b/ext/call.h index cffff00f..cceec9c4 100644 --- a/ext/call.h +++ b/ext/call.h @@ -109,11 +109,14 @@ class Op { typedef std::vector> OpVec; struct tag { - tag(Nan::Callback *callback, OpVec *ops, Call *call); + tag(Nan::Callback *callback, OpVec *ops, Call *call, + v8::Local call_value); ~tag(); Nan::Callback *callback; OpVec *ops; Call *call; + Nan::Persistent> + call_persist; }; v8::Local GetTagNodeValue(void *tag); diff --git a/ext/channel.cc b/ext/channel.cc index c795ff7f..1263cc0d 100644 --- a/ext/channel.cc +++ b/ext/channel.cc @@ -280,7 +280,7 @@ NAN_METHOD(Channel::WatchConnectivityState) { channel->wrapped_channel, last_state, MillisecondsToTimespec(deadline), GetCompletionQueue(), new struct tag(callback, - ops.release(), NULL)); + ops.release(), NULL, Nan::Null())); CompletionQueueNext(); } diff --git a/ext/server.cc b/ext/server.cc index ccb55aa5..f0920c84 100644 --- a/ext/server.cc +++ b/ext/server.cc @@ -193,7 +193,7 @@ NAN_METHOD(Server::RequestCall) { GetCompletionQueue(), GetCompletionQueue(), new struct tag(new Callback(info[0].As()), ops.release(), - NULL)); + NULL, Nan::Null())); if (error != GRPC_CALL_OK) { return Nan::ThrowError(nanErrorWithCode("requestCall failed", error)); } @@ -246,7 +246,7 @@ NAN_METHOD(Server::TryShutdown) { grpc_server_shutdown_and_notify( server->wrapped_server, GetCompletionQueue(), new struct tag(new Nan::Callback(info[0].As()), ops.release(), - NULL)); + NULL, Nan::Null())); CompletionQueueNext(); } diff --git a/ext/server_uv.cc b/ext/server_uv.cc index c5e5ca9f..82e7589f 100644 --- a/ext/server_uv.cc +++ b/ext/server_uv.cc @@ -118,7 +118,8 @@ void Server::ShutdownServer() { grpc_server_shutdown_and_notify( this->wrapped_server, GetCompletionQueue(), - new struct tag(new Callback(**shutdown_callback), ops.release(), NULL)); + new struct tag(new Callback(**shutdown_callback), ops.release(), NULL, + Nan::Null())); grpc_server_cancel_all_calls(this->wrapped_server); CompletionQueueNext(); this->wrapped_server = NULL; From 60a0ed4903b2604aae01b1cb67054e798521ac48 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 6 Apr 2017 13:54:37 -0700 Subject: [PATCH 2/2] Node: consolidate call destruction logic --- ext/call.cc | 18 ++++++++++-------- ext/call.h | 2 ++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/ext/call.cc b/ext/call.cc index 62f0130d..769f744b 100644 --- a/ext/call.cc +++ b/ext/call.cc @@ -515,16 +515,20 @@ void DestroyTag(void *tag) { delete tag_struct; } +void Call::DestroyCall() { + if (this->wrapped_call != NULL) { + grpc_call_destroy(this->wrapped_call); + this->wrapped_call = NULL; + } +} + Call::Call(grpc_call *call) : wrapped_call(call), pending_batches(0), has_final_op_completed(false) { } Call::~Call() { - if (wrapped_call != NULL) { - grpc_call_destroy(wrapped_call); - wrapped_call = NULL; - } + DestroyCall(); } void Call::Init(Local exports) { @@ -570,10 +574,8 @@ void Call::CompleteBatch(bool is_final_op) { this->has_final_op_completed = true; } this->pending_batches--; - if (this->has_final_op_completed && this->pending_batches == 0 && - this->wrapped_call != NULL) { - grpc_call_destroy(this->wrapped_call); - this->wrapped_call = NULL; + if (this->has_final_op_completed && this->pending_batches == 0) { + this->DestroyCall(); } } diff --git a/ext/call.h b/ext/call.h index cceec9c4..7fd03ca1 100644 --- a/ext/call.h +++ b/ext/call.h @@ -76,6 +76,8 @@ class Call : public Nan::ObjectWrap { Call(const Call &); Call &operator=(const Call &); + void DestroyCall(); + static NAN_METHOD(New); static NAN_METHOD(StartBatch); static NAN_METHOD(Cancel);