Skip to content

Commit

Permalink
valgrind: fix leaks of messgae_utils (XiaoMi#179)
Browse files Browse the repository at this point in the history
  • Loading branch information
Wu Tao authored and HuangWei committed Nov 5, 2018
1 parent bb674be commit f90f01f
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 43 deletions.
10 changes: 5 additions & 5 deletions include/dsn/cpp/message_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
namespace dsn {

/// Move the content inside message `m` into a blob.
inline blob move_message_to_blob(dsn::message_ex *m)
inline blob move_message_to_blob(message_ex *m)
{
rpc_read_stream reader(m);
return reader.get_buffer();
Expand All @@ -48,13 +48,13 @@ inline blob move_message_to_blob(dsn::message_ex *m)
/// however it passes a blob to ensure ownership safety instead of
/// passing simply a constant view.
/// MUST released manually later using dsn::message_ex::release_ref.
extern dsn::message_ex *
extern message_ex *
from_blob_to_received_msg(task_code rpc_code,
const blob &bb,
int thread_hash = 0,
uint64_t partition_hash = 0,
dsn_msg_serialize_format serialization_type = DSF_THRIFT_BINARY);
inline dsn::message_ex *
inline message_ex *
from_blob_to_received_msg(task_code rpc_code,
blob &&bb,
int thread_hash = 0,
Expand All @@ -68,7 +68,7 @@ from_blob_to_received_msg(task_code rpc_code,
/// It's useful for unit test, especially when we need to create a fake message
/// as test input.
template <typename T>
inline dsn::message_ex *from_thrift_request_to_received_message(const T &thrift_request,
inline message_ex *from_thrift_request_to_received_message(const T &thrift_request,
task_code tc)
{
binary_writer writer;
Expand All @@ -78,7 +78,7 @@ inline dsn::message_ex *from_thrift_request_to_received_message(const T &thrift_

/// Convert a blob into a thrift object.
template <typename T>
inline void from_blob_to_thrift(const dsn::blob &data, T &thrift_obj)
inline void from_blob_to_thrift(const blob &data, T &thrift_obj)
{
binary_reader reader(data);
unmarshall_thrift_binary(reader, thrift_obj);
Expand Down
2 changes: 2 additions & 0 deletions include/dsn/tool-api/command_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class command_manager : public ::dsn::utils::singleton<command_manager>
typedef std::function<std::string(const std::vector<std::string> &)> command_handler;
command_manager();

~command_manager();

dsn_handle_t register_command(const std::vector<std::string> &commands,
const std::string &help_one_line,
const std::string &help_long,
Expand Down
9 changes: 5 additions & 4 deletions include/dsn/utility/enum_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <map>
#include <string>
#include <mutex>
#include <memory>

// an invalid enum value must be provided so as to be the default value when parsing failed
#define ENUM_BEGIN2(type, name, invalid_value) \
Expand Down Expand Up @@ -106,13 +107,13 @@ class enum_helper_xxx
{
if (_instance == nullptr) {
static std::once_flag flag;
std::call_once(flag, [&]() { _instance = registor(); });
std::call_once(flag, [&]() { _instance.reset(registor()); });
}
return *_instance;
}

private:
static enum_helper_xxx *_instance;
static std::unique_ptr<enum_helper_xxx<TEnum>> _instance;

private:
TEnum _invalid;
Expand All @@ -121,6 +122,6 @@ class enum_helper_xxx
};

template <typename TEnum>
enum_helper_xxx<TEnum> *enum_helper_xxx<TEnum>::_instance = 0;
std::unique_ptr<enum_helper_xxx<TEnum>> enum_helper_xxx<TEnum>::_instance;

} // end namespace
} // namespace dsn
17 changes: 8 additions & 9 deletions src/core/core/command_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@
* THE SOFTWARE.
*/

/*
* Description:
* What is this file about?
*
* Revision history:
* xxxx-xx-xx, author, first version
* xxxx-xx-xx, author, fix bug about xxx
*/

#include <iostream>
#include <thread>
#include <sstream>
Expand Down Expand Up @@ -196,4 +187,12 @@ command_manager::command_manager()
return "repeat command completed";
});
}

command_manager::~command_manager()
{
for (command_instance *c : _commands) {
delete c;
}
}

} // namespace dsn
1 change: 0 additions & 1 deletion src/core/core/message_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ namespace dsn {
msg->header->client.thread_hash = thread_hash;
msg->header->client.partition_hash = partition_hash;
msg->header->context.u.serialize_format = serialization_type;
msg->add_ref(); // released by callers explicitly using dsn_msg_release
return msg;
}

Expand Down
18 changes: 10 additions & 8 deletions src/core/tests/message_utils_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#include <dsn/cpp/rpc_holder.h>
#include <gtest/gtest.h>

using namespace dsn;
namespace dsn {

DEFINE_TASK_CODE_RPC(RPC_CODE_FOR_TEST, TASK_PRIORITY_COMMON, THREAD_POOL_DEFAULT)

Expand All @@ -41,21 +41,21 @@ TEST(message_utils, msg_blob_convertion)
std::string data = "hello";

blob b(data.c_str(), 0, data.size());
dsn::message_ex *m = from_blob_to_received_msg(RPC_CODE_FOR_TEST, std::move(b));
message_ptr m = from_blob_to_received_msg(RPC_CODE_FOR_TEST, std::move(b));

ASSERT_EQ(m->header->body_length, data.size());
ASSERT_EQ(b.to_string(), move_message_to_blob(m).to_string());
ASSERT_EQ(b.to_string(), move_message_to_blob(m.get()).to_string());
}

TEST(message_utils, thrift_msg_convertion)
{
configuration_query_by_index_request request;
request.app_name = "haha";

dsn::message_ex *msg =
message_ptr msg =
from_thrift_request_to_received_message(request, RPC_CM_QUERY_PARTITION_CONFIG_BY_INDEX);

t_rpc rpc(msg);
t_rpc rpc(msg.get());
ASSERT_EQ(rpc.request().app_name, "haha");
}

Expand All @@ -64,11 +64,13 @@ TEST(message_utils, complex_convertion)
configuration_query_by_index_request request;
request.app_name = "haha";

dsn::message_ex *msg =
message_ptr msg =
from_thrift_request_to_received_message(request, RPC_CM_QUERY_PARTITION_CONFIG_BY_INDEX);
blob b = move_message_to_blob(msg);
blob b = move_message_to_blob(msg.get());
msg = from_blob_to_received_msg(RPC_CM_QUERY_PARTITION_CONFIG_BY_INDEX, std::move(b));

t_rpc rpc(msg);
t_rpc rpc(msg.get());
ASSERT_EQ(rpc.request().app_name, "haha");
}

} // namespace dsn
15 changes: 3 additions & 12 deletions src/core/tests/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@
* THE SOFTWARE.
*/

/*
* Description:
* Unit-test for rpc related code.
*
* Revision history:
* xxxx-xx-xx, author, first version
* xxxx-xx-xx, author, fix bug about xxx
*/

#include <vector>
#include <string>
#include <queue>
Expand Down Expand Up @@ -195,11 +186,11 @@ static void send_message(::dsn::rpc_address addr,
{
task_resp_queue q("response.queue");
for (int i = 0; i != repeat_times; ++i) {
dsn::message_ex *request = dsn::message_ex::create_request(RPC_TEST_STRING_COMMAND);
::dsn::marshall(request, command);
dsn::message_ptr request = dsn::message_ex::create_request(RPC_TEST_STRING_COMMAND);
::dsn::marshall(request.get(), command);
dsn::task_ptr resp_task = ::dsn::rpc::call(
addr,
request,
request.get(),
nullptr,
[&](error_code err, dsn::message_ex *request, dsn::message_ex *response) {
rpc_group_callback(
Expand Down
9 changes: 5 additions & 4 deletions src/core/tools/common/profiler_output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include <iostream>
#include <algorithm>
#include <dsn/toollet/profiler.h>
#include <dsn/utility/smart_pointers.h>
#include "profiler_header.h"

namespace dsn {
Expand All @@ -57,8 +58,8 @@ struct sort_node

extern task_spec_profiler *s_spec_profilers;
extern counter_info *counter_info_ptr[PREF_COUNTER_COUNT];
profiler_output_data_type *profiler_output_data =
new profiler_output_data_type(taskname_width, data_width, call_width);
auto profiler_output_data =
make_unique<profiler_output_data_type>(taskname_width, data_width, call_width);

static inline bool cmp(const sort_node &x, const sort_node &y)
{
Expand Down Expand Up @@ -394,5 +395,5 @@ void profiler_data_top(std::stringstream &ss,
}
delete[] _tmp;
}
}
}
} // namespace tools
} // namespace dsn

0 comments on commit f90f01f

Please sign in to comment.