From 17be589de0c41807b14fe47165e165c7d64deb9c Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 13 Feb 2015 10:19:10 -0800 Subject: Made changes based on comments --- src/node/ext/call.cc | 86 ++++++++++++++++++++------------------------------ src/node/ext/call.h | 15 +++++---- src/node/ext/server.cc | 6 ++-- 3 files changed, 46 insertions(+), 61 deletions(-) diff --git a/src/node/ext/call.cc b/src/node/ext/call.cc index 4401698b1e..18f40f2488 100644 --- a/src/node/ext/call.cc +++ b/src/node/ext/call.cc @@ -48,6 +48,7 @@ #include "timeval.h" using std::unique_ptr; +using std::shared_ptr; namespace grpc { namespace node { @@ -76,10 +77,8 @@ Persistent Call::constructor; Persistent Call::fun_tpl; -bool CreateMetadataArray( - Handle metadata, grpc_metadata_array *array, - std::vector > *string_handles, - std::vector > *handles) { +bool CreateMetadataArray(Handle metadata, grpc_metadata_array *array, + shared_ptr resources) { NanScope(); grpc_metadata_array_init(array); Handle keys(metadata->GetOwnPropertyNames()); @@ -95,7 +94,7 @@ bool CreateMetadataArray( for (unsigned int i = 0; i < keys->Length(); i++) { Handle current_key(keys->Get(i)->ToString()); NanUtf8String *utf8_key = new NanUtf8String(current_key); - string_handles->push_back(unique_ptr(utf8_key)); + resources->strings.push_back(unique_ptr(utf8_key)); Handle values = Local::Cast(metadata->Get(current_key)); for (unsigned int j = 0; j < values->Length(); j++) { Handle value = values->Get(j); @@ -106,12 +105,12 @@ bool CreateMetadataArray( current->value_length = Buffer::Length(value); Persistent handle; NanAssignPersistent(handle, value); - handles->push_back(unique_ptr( + resources->handles.push_back(unique_ptr( new PersistentHolder(handle))); } else if (value->IsString()) { Handle string_value = value->ToString(); NanUtf8String *utf8_value = new NanUtf8String(string_value); - string_handles->push_back(unique_ptr(utf8_value)); + resources->strings.push_back(unique_ptr(utf8_value)); current->value = **utf8_value; current->value_length = string_value->Length(); } else { @@ -168,13 +167,12 @@ class SendMetadataOp : public Op { return NanEscapeScope(NanTrue()); } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { if (!value->IsObject()) { return false; } grpc_metadata_array array; - if (!CreateMetadataArray(value->ToObject(), &array, strings, handles)) { + if (!CreateMetadataArray(value->ToObject(), &array, resources)) { return false; } out->data.send_initial_metadata.count = array.count; @@ -194,8 +192,7 @@ class SendMessageOp : public Op { return NanEscapeScope(NanTrue()); } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { if (!Buffer::HasInstance(value)) { return false; } @@ -204,7 +201,7 @@ class SendMessageOp : public Op { Handle temp = NanNew(); NanAssignPersistent(handle, temp); NanAssignPersistent(handle, value); - handles->push_back(unique_ptr( + resources->handles.push_back(unique_ptr( new PersistentHolder(handle))); return true; } @@ -221,8 +218,7 @@ class SendClientCloseOp : public Op { return NanEscapeScope(NanTrue()); } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { return true; } protected: @@ -238,8 +234,7 @@ class SendServerStatusOp : public Op { return NanEscapeScope(NanTrue()); } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { if (!value->IsObject()) { return false; } @@ -256,7 +251,7 @@ class SendServerStatusOp : public Op { grpc_metadata_array array; if (!CreateMetadataArray(server_status->Get(NanNew("metadata"))-> ToObject(), - &array, strings, handles)) { + &array, resources)) { return false; } out->data.send_status_from_server.trailing_metadata_count = array.count; @@ -266,7 +261,7 @@ class SendServerStatusOp : public Op { server_status->Get(NanNew("code"))->Uint32Value()); NanUtf8String *str = new NanUtf8String( server_status->Get(NanNew("details"))); - strings->push_back(unique_ptr(str)); + resources->strings.push_back(unique_ptr(str)); out->data.send_status_from_server.status_details = **str; return true; } @@ -292,8 +287,7 @@ class GetMetadataOp : public Op { } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { out->data.recv_initial_metadata = &recv_metadata; return true; } @@ -323,8 +317,7 @@ class ReadMessageOp : public Op { } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { out->data.recv_message = &recv_message; return true; } @@ -352,8 +345,7 @@ class ClientStatusOp : public Op { } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { out->data.recv_status_on_client.trailing_metadata = &metadata_array; out->data.recv_status_on_client.status = &status; out->data.recv_status_on_client.status_details = &status_details; @@ -390,8 +382,7 @@ class ServerCloseResponseOp : public Op { } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { out->data.recv_close_on_server.cancelled = &cancelled; return true; } @@ -406,19 +397,13 @@ class ServerCloseResponseOp : public Op { }; tag::tag(NanCallback *callback, std::vector > *ops, - std::vector > *handles, - std::vector > *strings) : - callback(callback), ops(ops), handles(handles), strings(strings){ + shared_ptr resources) : + callback(callback), ops(ops), resources(resources){ } + tag::~tag() { delete callback; delete ops; - if (handles != NULL) { - delete handles; - } - if (strings != NULL) { - delete strings; - } } Handle GetTagNodeValue(void *tag) { @@ -542,17 +527,14 @@ NAN_METHOD(Call::StartBatch) { Handle callback_func = args[1].As(); NanCallback *callback = new NanCallback(callback_func); Call *call = ObjectWrap::Unwrap(args.This()); - std::vector > *handles = - new std::vector >(); - std::vector > *strings = - new std::vector >(); + shared_ptr resources(new Resources); Handle obj = args[0]->ToObject(); Handle keys = obj->GetOwnPropertyNames(); size_t nops = keys->Length(); grpc_op *ops = new grpc_op[nops]; std::vector > *op_vector = new std::vector >(); for (unsigned int i = 0; i < nops; i++) { - Op *op; + unique_ptr op; if (!keys->Get(i)->IsUint32()) { return NanThrowError( "startBatch's first argument's keys must be integers"); @@ -561,40 +543,40 @@ NAN_METHOD(Call::StartBatch) { ops[i].op = static_cast(type); switch (type) { case GRPC_OP_SEND_INITIAL_METADATA: - op = new SendMetadataOp(); + op.reset(new SendMetadataOp()); break; case GRPC_OP_SEND_MESSAGE: - op = new SendMessageOp(); + op.reset(new SendMessageOp()); break; case GRPC_OP_SEND_CLOSE_FROM_CLIENT: - op = new SendClientCloseOp(); + op.reset(new SendClientCloseOp()); break; case GRPC_OP_SEND_STATUS_FROM_SERVER: - op = new SendServerStatusOp(); + op.reset(new SendServerStatusOp()); break; case GRPC_OP_RECV_INITIAL_METADATA: - op = new GetMetadataOp(); + op.reset(new GetMetadataOp()); break; case GRPC_OP_RECV_MESSAGE: - op = new ReadMessageOp(); + op.reset(new ReadMessageOp()); break; case GRPC_OP_RECV_STATUS_ON_CLIENT: - op = new ClientStatusOp(); + op.reset(new ClientStatusOp()); break; case GRPC_OP_RECV_CLOSE_ON_SERVER: - op = new ServerCloseResponseOp(); + op.reset(new ServerCloseResponseOp()); break; default: return NanThrowError("Argument object had an unrecognized key"); } - if (!op->ParseOp(obj->Get(type), &ops[i], strings, handles)) { + if (!op->ParseOp(obj->Get(type), &ops[i], resources)) { return NanThrowTypeError("Incorrectly typed arguments to startBatch"); } - op_vector->push_back(unique_ptr(op)); + op_vector->push_back(std::move(op)); } grpc_call_error error = grpc_call_start_batch( call->wrapped_call, ops, nops, new struct tag( - callback, op_vector, handles, strings)); + callback, op_vector, resources)); delete ops; if (error != GRPC_CALL_OK) { return NanThrowError("startBatch failed", error); diff --git a/src/node/ext/call.h b/src/node/ext/call.h index f443a04637..4074f1509b 100644 --- a/src/node/ext/call.h +++ b/src/node/ext/call.h @@ -47,6 +47,7 @@ namespace grpc { namespace node { using std::unique_ptr; +using std::shared_ptr; v8::Handle ParseMetadata(const grpc_metadata_array *metadata_array); @@ -64,12 +65,16 @@ class PersistentHolder { v8::Persistent persist; }; +struct Resources { + std::vector > strings; + std::vector > handles; +}; + class Op { public: virtual v8::Handle GetNodeValue() const = 0; virtual bool ParseOp(v8::Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) = 0; + shared_ptr resources) = 0; v8::Handle GetOpType() const; protected: @@ -78,13 +83,11 @@ class Op { struct tag { tag(NanCallback *callback, std::vector > *ops, - std::vector > *handles, - std::vector > *strings); + shared_ptr resources); ~tag(); NanCallback *callback; std::vector > *ops; - std::vector > *handles; - std::vector > *strings; + shared_ptr resources; }; v8::Handle GetTagNodeValue(void *tag); diff --git a/src/node/ext/server.cc b/src/node/ext/server.cc index 51904479a9..6a4a951121 100644 --- a/src/node/ext/server.cc +++ b/src/node/ext/server.cc @@ -101,8 +101,7 @@ class NewCallOp : public Op { } bool ParseOp(Handle value, grpc_op *out, - std::vector > *strings, - std::vector > *handles) { + shared_ptr resources) { return true; } @@ -230,7 +229,8 @@ NAN_METHOD(Server::RequestCall) { grpc_call_error error = grpc_server_request_call( server->wrapped_server, &op->call, &op->details, &op->request_metadata, CompletionQueueAsyncWorker::GetQueue(), - new struct tag(new NanCallback(args[0].As()), ops, NULL, NULL)); + new struct tag(new NanCallback(args[0].As()), ops, + shared_ptr(nullptr))); if (error != GRPC_CALL_OK) { return NanThrowError("requestCall failed", error); } -- cgit v1.2.3