From f84de73e6620a0ee494368ac65a5ecfcf8397b2c Mon Sep 17 00:00:00 2001 From: John Jones Date: Fri, 23 Mar 2018 17:34:07 -0500 Subject: [PATCH 1/3] added test to demonstrate issue --- src/asio.cpp | 9 ++++++++- src/network/tcp_socket.cpp | 2 +- tests/CMakeLists.txt | 1 + tests/io/tcp_test.cpp | 12 ++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 tests/io/tcp_test.cpp diff --git a/src/asio.cpp b/src/asio.cpp index 1313ed7..d596e84 100644 --- a/src/asio.cpp +++ b/src/asio.cpp @@ -99,6 +99,7 @@ namespace fc { default_io_service_scope() { + cleanup_complete = false; io = new boost::asio::io_service(); the_work = new boost::asio::io_service::work(*io); for( int i = 0; i < 8; ++i ) { @@ -146,10 +147,16 @@ namespace fc { for( auto asio_thread : asio_threads ) { delete asio_thread; } + cleanup_complete = true; } ~default_io_service_scope() - {} + { + if (!cleanup_complete) + cleanup(); + } + private: + bool cleanup_complete; }; /// If cleanup is true, do not use the return value; it is a null reference diff --git a/src/network/tcp_socket.cpp b/src/network/tcp_socket.cpp index 1c980fc..57b9adf 100644 --- a/src/network/tcp_socket.cpp +++ b/src/network/tcp_socket.cpp @@ -90,7 +90,7 @@ namespace fc { tcp_socket::tcp_socket(){}; - tcp_socket::~tcp_socket(){}; + tcp_socket::~tcp_socket() {} void tcp_socket::flush() {} void tcp_socket::close() { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 40f3fe6..ae6ea92 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -51,6 +51,7 @@ add_executable( all_tests all_tests.cpp crypto/sha_tests.cpp io/json_tests.cpp io/stream_tests.cpp + io/tcp_test.cpp network/http/websocket_test.cpp thread/task_cancel.cpp thread/thread_tests.cpp diff --git a/tests/io/tcp_test.cpp b/tests/io/tcp_test.cpp new file mode 100644 index 0000000..535e607 --- /dev/null +++ b/tests/io/tcp_test.cpp @@ -0,0 +1,12 @@ +#include + +#include + +BOOST_AUTO_TEST_SUITE(tcp_tests) + +BOOST_AUTO_TEST_CASE(tcpconstructor_test) +{ + fc::tcp_socket socket; +} + +BOOST_AUTO_TEST_SUITE_END() From cb2965991f2229e92fdd082f15cfd892a165533e Mon Sep 17 00:00:00 2001 From: John Jones Date: Fri, 4 May 2018 07:47:37 -0500 Subject: [PATCH 2/3] Added comment to clarify test purpose --- tests/io/tcp_test.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/io/tcp_test.cpp b/tests/io/tcp_test.cpp index 535e607..03d46ee 100644 --- a/tests/io/tcp_test.cpp +++ b/tests/io/tcp_test.cpp @@ -4,6 +4,11 @@ BOOST_AUTO_TEST_SUITE(tcp_tests) +/*** + * Running this test through valgrind will show + * a memory leak due to lack of logic in destructor. + * See https://github.com/bitshares/bitshares-fc/blob/51688042b0b9f99f03224f54fb937fe024fe5ced/src/asio.cpp#L152 + */ BOOST_AUTO_TEST_CASE(tcpconstructor_test) { fc::tcp_socket socket; From 3661e835f8742d150677400825d8b4fc68fb5e4d Mon Sep 17 00:00:00 2001 From: John Jones Date: Fri, 4 May 2018 11:05:20 -0500 Subject: [PATCH 3/3] Removed unused parameter --- include/fc/asio.hpp | 2 +- src/asio.cpp | 23 ++++++----------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/include/fc/asio.hpp b/include/fc/asio.hpp index 9310774..6b5b4b3 100644 --- a/include/fc/asio.hpp +++ b/include/fc/asio.hpp @@ -72,7 +72,7 @@ namespace asio { * This IO service is automatically running in its own thread to service asynchronous * requests without blocking any other threads. */ - boost::asio::io_service& default_io_service(bool cleanup = false); + boost::asio::io_service& default_io_service(); /** * @brief wraps boost::asio::async_read diff --git a/src/asio.cpp b/src/asio.cpp index d596e84..81724ca 100644 --- a/src/asio.cpp +++ b/src/asio.cpp @@ -99,7 +99,6 @@ namespace fc { default_io_service_scope() { - cleanup_complete = false; io = new boost::asio::io_service(); the_work = new boost::asio::io_service::work(*io); for( int i = 0; i < 8; ++i ) { @@ -136,7 +135,7 @@ namespace fc { } } - void cleanup() + ~default_io_service_scope() { delete the_work; io->stop(); @@ -147,25 +146,15 @@ namespace fc { for( auto asio_thread : asio_threads ) { delete asio_thread; } - cleanup_complete = true; } - - ~default_io_service_scope() - { - if (!cleanup_complete) - cleanup(); - } - private: - bool cleanup_complete; }; - /// If cleanup is true, do not use the return value; it is a null reference - boost::asio::io_service& default_io_service(bool cleanup) { + /*** + * @brief create an io_service + * @returns the io_service + */ + boost::asio::io_service& default_io_service() { static default_io_service_scope fc_asio_service[1]; - if (cleanup) { - for( int i = 0; i < 1; ++i ) - fc_asio_service[i].cleanup(); - } return *fc_asio_service[0].io; }