From 77e06b2f8617f190fbd14e47238ce8d304118d2c Mon Sep 17 00:00:00 2001
From: murgatroid99 <mlumish@google.com>
Date: Thu, 6 Apr 2017 10:37:44 -0700
Subject: [PATCH] Node: fix leak of sent metadata
---
src/node/ext/call.cc | 48 +++++++++++++++++++++++---------
src/node/ext/call.h | 2 ++
src/node/ext/call_credentials.cc | 2 ++
3 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/src/node/ext/call.cc b/src/node/ext/call.cc
index 5c31115854..c77d6a3379 100644
--- a/src/node/ext/call.cc
+++ b/src/node/ext/call.cc
@@ -99,7 +99,6 @@ Local<Value> nanErrorWithCode(const char *msg, grpc_call_error code) {
bool CreateMetadataArray(Local<Object> metadata, grpc_metadata_array *array) {
HandleScope scope;
- grpc_metadata_array_init(array);
Local<Array> keys = Nan::GetOwnPropertyNames(metadata).ToLocalChecked();
for (unsigned int i = 0; i < keys->Length(); i++) {
Local<String> current_key = Nan::To<String>(
@@ -111,18 +110,20 @@ bool CreateMetadataArray(Local<Object> metadata, grpc_metadata_array *array) {
array->capacity += Local<Array>::Cast(value_array)->Length();
}
array->metadata = reinterpret_cast<grpc_metadata*>(
- gpr_malloc(array->capacity * sizeof(grpc_metadata)));
+ gpr_zalloc(array->capacity * sizeof(grpc_metadata)));
for (unsigned int i = 0; i < keys->Length(); i++) {
Local<String> current_key(Nan::To<String>(keys->Get(i)).ToLocalChecked());
Local<Array> values = Local<Array>::Cast(
Nan::Get(metadata, current_key).ToLocalChecked());
- grpc_slice key_slice = grpc_slice_intern(CreateSliceFromString(current_key));
+ grpc_slice key_slice = CreateSliceFromString(current_key);
+ grpc_slice key_intern_slice = grpc_slice_intern(key_slice);
+ grpc_slice_unref(key_slice);
for (unsigned int j = 0; j < values->Length(); j++) {
Local<Value> value = Nan::Get(values, j).ToLocalChecked();
grpc_metadata *current = &array->metadata[array->count];
- current->key = key_slice;
+ current->key = key_intern_slice;
// Only allow binary headers for "-bin" keys
- if (grpc_is_binary_header(key_slice)) {
+ if (grpc_is_binary_header(key_intern_slice)) {
if (::node::Buffer::HasInstance(value)) {
current->value = CreateSliceFromBuffer(value);
} else {
@@ -142,6 +143,14 @@ bool CreateMetadataArray(Local<Object> metadata, grpc_metadata_array *array) {
return true;
}
+void DestroyMetadataArray(grpc_metadata_array *array) {
+ for (size_t i = 0; i < array->count; i++) {
+ // Don't unref keys because they are interned
+ grpc_slice_unref(array->metadata[i].value);
+ }
+ grpc_metadata_array_destroy(array);
+}
+
Local<Value> ParseMetadata(const grpc_metadata_array *metadata_array) {
EscapableHandleScope scope;
grpc_metadata *metadata_elements = metadata_array->metadata;
@@ -179,6 +188,12 @@ Op::~Op() {
class SendMetadataOp : public Op {
public:
+ SendMetadataOp() {
+ grpc_metadata_array_init(&send_metadata);
+ }
+ ~SendMetadataOp() {
+ DestroyMetadataArray(&send_metadata);
+ }
Local<Value> GetNodeValue() const {
EscapableHandleScope scope;
return scope.Escape(Nan::True());
@@ -187,17 +202,16 @@ class SendMetadataOp : public Op {
if (!value->IsObject()) {
return false;
}
- grpc_metadata_array array;
MaybeLocal<Object> maybe_metadata = Nan::To<Object>(value);
if (maybe_metadata.IsEmpty()) {
return false;
}
if (!CreateMetadataArray(maybe_metadata.ToLocalChecked(),
- &array)) {
+ &send_metadata)) {
return false;
}
- out->data.send_initial_metadata.count = array.count;
- out->data.send_initial_metadata.metadata = array.metadata;
+ out->data.send_initial_metadata.count = send_metadata.count;
+ out->data.send_initial_metadata.metadata = send_metadata.metadata;
return true;
}
bool IsFinalOp() {
@@ -207,6 +221,8 @@ class SendMetadataOp : public Op {
std::string GetTypeString() const {
return "send_metadata";
}
+ private:
+ grpc_metadata_array send_metadata;
};
class SendMessageOp : public Op {
@@ -272,8 +288,12 @@ class SendClientCloseOp : public Op {
class SendServerStatusOp : public Op {
public:
+ SendServerStatusOp() {
+ grpc_metadata_array_init(&status_metadata);
+ }
~SendServerStatusOp() {
grpc_slice_unref(details);
+ DestroyMetadataArray(&status_metadata);
}
Local<Value> GetNodeValue() const {
EscapableHandleScope scope;
@@ -313,12 +333,13 @@ class SendServerStatusOp : public Op {
}
Local<String> details = Nan::To<String>(
maybe_details.ToLocalChecked()).ToLocalChecked();
- grpc_metadata_array array;
- if (!CreateMetadataArray(metadata, &array)) {
+ if (!CreateMetadataArray(metadata, &status_metadata)) {
return false;
}
- out->data.send_status_from_server.trailing_metadata_count = array.count;
- out->data.send_status_from_server.trailing_metadata = array.metadata;
+ out->data.send_status_from_server.trailing_metadata_count =
+ status_metadata.count;
+ out->data.send_status_from_server.trailing_metadata =
+ status_metadata.metadata;
out->data.send_status_from_server.status =
static_cast<grpc_status_code>(code);
this->details = CreateSliceFromString(details);
@@ -335,6 +356,7 @@ class SendServerStatusOp : public Op {
private:
grpc_slice details;
+ grpc_metadata_array status_metadata;
};
class GetMetadataOp : public Op {
diff --git a/src/node/ext/call.h b/src/node/ext/call.h
index cffff00fce..fe2abab9ee 100644
--- a/src/node/ext/call.h
+++ b/src/node/ext/call.h
@@ -58,6 +58,8 @@ v8::Local<v8::Value> ParseMetadata(const grpc_metadata_array *metadata_array);
bool CreateMetadataArray(v8::Local<v8::Object> metadata,
grpc_metadata_array *array);
+void DestroyMetadataArray(grpc_metadata_array *array);
+
/* Wrapper class for grpc_call structs. */
class Call : public Nan::ObjectWrap {
public:
diff --git a/src/node/ext/call_credentials.cc b/src/node/ext/call_credentials.cc
index afcc363131..5bd4bdcd5a 100644
--- a/src/node/ext/call_credentials.cc
+++ b/src/node/ext/call_credentials.cc
@@ -211,6 +211,7 @@ NAN_METHOD(PluginCallback) {
Utf8String details_utf8_str(info[1]);
char *details = *details_utf8_str;
grpc_metadata_array array;
+ grpc_metadata_array_init(&array);
Local<Object> callback_data = Nan::To<Object>(info[3]).ToLocalChecked();
if (!CreateMetadataArray(Nan::To<Object>(info[2]).ToLocalChecked(),
&array)){
@@ -226,6 +227,7 @@ NAN_METHOD(PluginCallback) {
Nan::New("user_data").ToLocalChecked()
).ToLocalChecked().As<External>()->Value();
cb(user_data, array.metadata, array.count, code, details);
+ DestroyMetadataArray(&array);
}
NAUV_WORK_CB(SendPluginCallback) {
--
GitLab