Skip to content

Commit

Permalink
[c++ grpc] Fix multi-slice deser. memory leak
Browse files Browse the repository at this point in the history
The caller of grpc_byte_buffer_reader_next is responsible for calling
grpc_slice_unref on the slice. However, SerializationTraits::Deserialize
was only calling grpc_slice_unref on the last slice. Now, it is called
on each slice.

Also fixes a potential bug that would call grpc_slice_unref on an
uninitialized slice.
  • Loading branch information
chwarr committed Oct 30, 2017
1 parent 84ff6a9 commit 4334416
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ different versioning scheme, following the Haskell community's
* C# NuGet version: TBD
* C# Comm NuGet version: TBD

### C++ ###
* Fixed a memory leak when deserializing Bond-over-gRPC messages that were
stored as multiple slices.

## 7.0.1: 2017-10-26 ##
* `gbc` & compiler library: 0.10.1.0
* IDL core version: 2.0
Expand Down
12 changes: 6 additions & 6 deletions cpp/inc/bond/ext/grpc/bond_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <grpc++/impl/codegen/serialization_traits.h>
#include <grpc++/impl/codegen/status.h>
#include <grpc++/impl/codegen/status_code_enum.h>
#include <grpc++/support/slice.h>
#include <grpc/impl/codegen/byte_buffer_reader.h>
#include <grpc/impl/codegen/slice.h>

Expand Down Expand Up @@ -71,7 +72,7 @@ namespace grpc {

boost::shared_ptr<char[]> buff = boost::make_shared_noinit<char[]>(bufferSize);

// TODO: exception safety of reader, s, buffer
// TODO: exception safety of reader, buffer
grpc_byte_buffer_reader reader;
if (!grpc_byte_buffer_reader_init(&reader, buffer))
{
Expand All @@ -81,16 +82,15 @@ namespace grpc {

char* dest = buff.get();

grpc_slice s;
while (grpc_byte_buffer_reader_next(&reader, &s) != 0)
for (grpc_slice grpc_s; grpc_byte_buffer_reader_next(&reader, &grpc_s) != 0;)
{
std::memcpy(dest, GRPC_SLICE_START_PTR(s), GRPC_SLICE_LENGTH(s));
dest += GRPC_SLICE_LENGTH(s);
grpc::Slice s{ grpc_s, grpc::Slice::STEAL_REF };
std::memcpy(dest, s.begin(), s.size());
dest += s.size();
}

BOOST_ASSERT(dest == buff.get() + bufferSize);

grpc_slice_unref(s);
grpc_byte_buffer_reader_destroy(&reader);
grpc_byte_buffer_destroy(buffer);

Expand Down

0 comments on commit 4334416

Please sign in to comment.