Skip to content

Commit

Permalink
rdsn: fix asio_rpc_session bug which may cause double free (#115)
Browse files Browse the repository at this point in the history
  • Loading branch information
qinzuoyan authored Jun 29, 2018
1 parent ea6541c commit 044ce72
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 14 deletions.
1 change: 1 addition & 0 deletions include/dsn/c/api_utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#pragma once

#include <dsn/c/api_common.h>
#include <dsn/utility/ports.h>

/*!
@defgroup logging Logging Service
Expand Down
16 changes: 11 additions & 5 deletions include/dsn/utility/autoref_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,18 @@ class ref_counter

virtual ~ref_counter()
{
if (_magic != 0xdeadbeef) { // 3735928559
assert(!"memory corrupted, could be double free or others");
} else {
_magic = 0xfacedead; // 4207861421
}
// 0xdeadbeef: 3735928559
assert(_magic == 0xdeadbeef);

// 0xfacedead: 4207861421
_magic = 0xfacedead;
}

void add_ref()
{
// 0xdeadbeef: 3735928559
assert(_magic == 0xdeadbeef);

// Increasing the reference counter can always be done with memory_order_relaxed:
// New references to an object can only be formed from an existing reference,
// and passing an existing reference from one thread to another must already provide any
Expand All @@ -64,6 +67,9 @@ class ref_counter

void release_ref()
{
// 0xdeadbeef: 3735928559
assert(_magic == 0xdeadbeef);

// It is important to enforce any possible access to the object in one thread
//(through an existing reference) to happen before deleting the object in a different
// thread.
Expand Down
9 changes: 3 additions & 6 deletions include/dsn/utility/endians.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#pragma once

#include <cstdint>
#include <cassert>
#include <endian.h>
#include <dsn/service_api_c.h>
#include <dsn/utility/string_view.h>

namespace dsn {
Expand Down Expand Up @@ -57,7 +57,7 @@ class data_output
void ensure(size_t sz)
{
size_t cap = _end - _ptr;
dassert(cap >= sz, "capacity %zu is not enough for %zu", cap, sz);
assert(cap >= sz);
}

private:
Expand Down Expand Up @@ -107,10 +107,7 @@ class data_input
_size -= sz;
}

void ensure(size_t sz)
{
dassert(_size >= sz, " content(%zu) is not enough for reading %zu size", _size, sz);
}
void ensure(size_t sz) { assert(_size >= sz); }

private:
const char *_p{nullptr};
Expand Down
5 changes: 4 additions & 1 deletion src/core/tools/common/asio_net_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ void asio_network_provider::do_accept()
(std::shared_ptr<boost::asio::ip::tcp::socket> &)socket,
null_parser,
false);
this->on_server_session_accepted(s);
on_server_session_accepted(s);

// we should start read immediately after the rpc session is completely created.
s->start_read_next();
}

do_accept();
Expand Down
2 changes: 0 additions & 2 deletions src/core/tools/common/asio_rpc_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,6 @@ asio_rpc_session::asio_rpc_session(asio_network_provider &net,
: rpc_session(net, remote_addr, parser, is_client), _socket(socket)
{
set_options();
if (!is_client)
start_read_next();
}

void asio_rpc_session::on_failure(bool is_write)
Expand Down
1 change: 1 addition & 0 deletions src/core/tools/common/thrift_message_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ dsn::message_ex *thrift_message_parser::parse_message(const thrift_message_heade

dsn_hdr->id = seqid;
strncpy(dsn_hdr->rpc_name, fname.c_str(), DSN_MAX_TASK_CODE_NAME_LENGTH);
dsn_hdr->rpc_name[DSN_MAX_TASK_CODE_NAME_LENGTH - 1] = '\0';
dsn_hdr->gpid.set_app_id(thrift_header.app_id);
dsn_hdr->gpid.set_partition_index(thrift_header.partition_index);
dsn_hdr->client.timeout_ms = thrift_header.client_timeout;
Expand Down

0 comments on commit 044ce72

Please sign in to comment.