fix(lsan): memory leak in perf_counter_atomic.h #384

foreverneverer · 2020-01-21T09:11:36Z

Coredump

==25314==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40264 byte(s) in 1 object(s) allocated from:
    #0 0x7ff54859d532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x545989 in dsn::perf_counters::new_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:185
    #2 0x548fba in dsn::perf_counters::get_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*, bool) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:118
    #3 0x46ef2e in dsn::perf_counter_wrapper::init_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:99
    #4 0x46ef2e in dsn::perf_counter_http_service_test_get_perf_counter_Test::TestBody() /home/mi/work/PegasusDB/pegasus/rdsn/src/dist/http/test/perf_counter_http_service_test.cpp:36
    #5 0x6dcb6c in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6dcb6c)
    #6 0x6d6170 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6170)
    #7 0x6b9761 in testing::Test::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6b9761)
    #8 0x6ba0f9 in testing::TestInfo::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba0f9)
    #9 0x6ba7ec in testing::TestCase::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba7ec)
    #10 0x6c1933 in testing::internal::UnitTestImpl::RunAllTests() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c1933)
    #11 0x6de144 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6de144)
    #12 0x6d6fb2 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6fb2)
    #13 0x6c03cf in testing::UnitTest::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c03cf)
    #14 0x6eb034 in RUN_ALL_TESTS() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eb034)
    #15 0x6eafce in main (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eafce)
    #16 0x7ff546c3982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Indirect leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7ff54859d532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x545c4f in dsn::perf_counter_number_percentile_atomic::perf_counter_number_percentile_atomic(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counter_atomic.h:214
    #2 0x545c4f in dsn::perf_counters::new_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:185
    #3 0x548fba in dsn::perf_counters::get_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*, bool) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:118
    #4 0x46ef2e in dsn::perf_counter_wrapper::init_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:99
    #5 0x46ef2e in dsn::perf_counter_http_service_test_get_perf_counter_Test::TestBody() /home/mi/work/PegasusDB/pegasus/rdsn/src/dist/http/test/perf_counter_http_service_test.cpp:36
    #6 0x6dcb6c in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6dcb6c)
    #7 0x6d6170 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6170)
    #8 0x6b9761 in testing::Test::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6b9761)
    #9 0x6ba0f9 in testing::TestInfo::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba0f9)
    #10 0x6ba7ec in testing::TestCase::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba7ec)
    #11 0x6c1933 in testing::internal::UnitTestImpl::RunAllTests() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c1933)
    #12 0x6de144 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6de144)
    #13 0x6d6fb2 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6fb2)
    #14 0x6c03cf in testing::UnitTest::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c03cf)
    #15 0x6eb034 in RUN_ALL_TESTS() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eb034)
    #16 0x6eafce in main (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eafce)
    #17 0x7ff546c3982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Indirect leak of 25 byte(s) in 1 object(s) allocated from:
    #0 0x7ff54859d532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x7ff547614eec  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x112eec)
    #2 0x7ffd667b34af  (<unknown module>)
    #3 0x7ffd667b343f  (<unknown module>)

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7ff54859d532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x545e14 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > >*>(boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > >*) /usr/include/c++/5/bits/shared_ptr_base.h:574
    #2 0x545e14 in std::__shared_ptr<boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > >, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > > >(boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > >*) /usr/include/c++/5/bits/shared_ptr_base.h:882
    #3 0x545e14 in void std::__shared_ptr<boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > >, (__gnu_cxx::_Lock_policy)2>::reset<boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > > >(boost::asio::basic_deadline_timer<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime>, boost::asio::deadline_timer_service<boost::posix_time::ptime, boost::asio::time_traits<boost::posix_time::ptime> > >*) /usr/include/c++/5/bits/shared_ptr_base.h:1030
    #4 0x545e14 in dsn::perf_counter_number_percentile_atomic::perf_counter_number_percentile_atomic(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counter_atomic.h:214
    #5 0x545e14 in dsn::perf_counters::new_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:185
    #6 0x548fba in dsn::perf_counters::get_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*, bool) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:118
    #7 0x46ef2e in dsn::perf_counter_wrapper::init_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:99
    #8 0x46ef2e in dsn::perf_counter_http_service_test_get_perf_counter_Test::TestBody() /home/mi/work/PegasusDB/pegasus/rdsn/src/dist/http/test/perf_counter_http_service_test.cpp:36
    #9 0x6dcb6c in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6dcb6c)
    #10 0x6d6170 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6170)
    #11 0x6b9761 in testing::Test::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6b9761)
    #12 0x6ba0f9 in testing::TestInfo::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba0f9)
    #13 0x6ba7ec in testing::TestCase::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba7ec)
    #14 0x6c1933 in testing::internal::UnitTestImpl::RunAllTests() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c1933)
    #15 0x6de144 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6de144)
    #16 0x6d6fb2 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6fb2)
    #17 0x6c03cf in testing::UnitTest::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c03cf)
    #18 0x6eb034 in RUN_ALL_TESTS() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eb034)
    #19 0x6eafce in main (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eafce)
    #20 0x7ff546c3982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Indirect leak of 17 byte(s) in 1 object(s) allocated from:
    #0 0x7ff54859d532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x5430c3 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) /usr/include/c++/5/bits/basic_string.tcc:223
    #2 0x552dd8 in dsn::perf_counter::perf_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/include/dsn/perf_counter/perf_counter.h:72
    #3 0x5459ae in dsn::perf_counter_number_percentile_atomic::perf_counter_number_percentile_atomic(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counter_atomic.h:199
    #4 0x5459ae in dsn::perf_counters::new_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:185
    #5 0x548fba in dsn::perf_counters::get_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*, bool) /home/mi/work/PegasusDB/pegasus/rdsn/src/core/perf_counter/perf_counters.cpp:118
    #6 0x46ef2e in dsn::perf_counter_wrapper::init_global_counter(char const*, char const*, char const*, dsn_perf_counter_type_t, char const*) /home/mi/work/PegasusDB/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:99
    #7 0x46ef2e in dsn::perf_counter_http_service_test_get_perf_counter_Test::TestBody() /home/mi/work/PegasusDB/pegasus/rdsn/src/dist/http/test/perf_counter_http_service_test.cpp:36
    #8 0x6dcb6c in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6dcb6c)
    #9 0x6d6170 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6170)
    #10 0x6b9761 in testing::Test::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6b9761)
    #11 0x6ba0f9 in testing::TestInfo::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba0f9)
    #12 0x6ba7ec in testing::TestCase::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6ba7ec)
    #13 0x6c1933 in testing::internal::UnitTestImpl::RunAllTests() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c1933)
    #14 0x6de144 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6de144)
    #15 0x6d6fb2 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6d6fb2)
    #16 0x6c03cf in testing::UnitTest::Run() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6c03cf)
    #17 0x6eb034 in RUN_ALL_TESTS() (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eb034)
    #18 0x6eafce in main (/home/mi/work/PegasusDB/pegasus/rdsn/builder/src/dist/http/test/dsn_http_test+0x6eafce)
    #19 0x7ff546c3982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 40394 byte(s) leaked in 5 allocation(s).

Reason & Solution

rdsn/src/core/perf_counter/perf_counter_atomic.h

Lines 213 to 221 in 766edef

    
               _timer.reset(new boost::asio::deadline_timer(tools::shared_io_service::instance().ios)); 
        
               _timer->expires_from_now( 
        
                   boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1)); 
        
               this->add_ref(); 
        
               _timer->async_wait(std::bind( 
        
                   &perf_counter_number_percentile_atomic::on_timer, this, _timer, std::placeholders::_1)); 
        
           } 
        
           ~perf_counter_number_percentile_atomic(void) { _timer->cancel(); }

The object is added reference count by this.add_ref() in line 216, result in that it will be not released before the async callback on_timer() complete.

After deleting the this.add_ref(), on_timer() will not excute after perf_counter_number_percentile_atomic is released because the ~perf_counter_number_percentile_atomic(void) { _timer->cancel(); } will cancel the timer task. So we don't need worry about the case that using the released object.

Change & Potential problem

In the old version, after the thread perf_counter_number_percentile_atomic is in is terminated, the timer won’t stop until the timer occurs error (for example, being canceled).

In this PR, it will stop the timer task after the thread perf_counter_number_percentile_atomic is in is terminated, we need consider whether this change haves an effect on the result of timer task for calc(ctx)(in line 420).

levy5307 · 2020-01-21T10:26:26Z

src/core/perf_counter/perf_counter_atomic.h

-                                            std::placeholders::_1));
-            }
+            boost::shared_ptr<compute_context> ctx(new compute_context());
+            calc(ctx);


这里是不是可以将两句合并成一句

Suggested change

calc(ctx);

calc(boost::make_shared<compute_context>());

Note: calc(boost::make_shared<compute_context>()) result in the argument of calc(boost::shared_ptr<compute_context> &ctx) (also contain other method) is set to const .

vagetablechicken · 2020-01-21T10:29:01Z

After deleting the this.add_ref(), on_timer() will not excute after perf_counter_number_percentile_atomic is released because the ~perf_counter_number_percentile_atomic(void) { _timer->cancel(); } will cancel the timer task. So we don't need worry about the case that using the released object.

Why the original code needs add/release_ref? The logic is the same?

foreverneverer · 2020-01-21T11:19:32Z

After deleting the this.add_ref(), on_timer() will not excute after perf_counter_number_percentile_atomic is released because the ~perf_counter_number_percentile_atomic(void) { _timer->cancel(); } will cancel the timer task. So we don't need worry about the case that using the released object.

Why the original code needs add/release_ref? The logic is the same?

I think that it's for avoiding to use the released perf_counter_number_percentile_atomic object in on timer(), but it's unnecessary as explained above.

vagetablechicken · 2020-01-22T02:26:40Z

ref 5422c63
add/release_ref may be a bug fix

foreverneverer · 2020-01-22T03:22:05Z

ref 5422c63
add/release_ref may be a bug fix

Right, we need consider it. I update the description of the pr:

Change & Potential problem

In the old version, after the thread perf_counter_number_percentile_atomic is in is terminated, the timer won’t stop until the timer occurs error (for example, being canceled).

In this PR, it will stop the timer task after the thread perf_counter_number_percentile_atomic is in is terminated, we need consider whether this change haves an effect on the result of timer task for calc(ctx) (in line 420).

neverchanje

Change & Potential problem

In the old version, after the thread perf_counter_number_percentile_atomic is in is terminated, the timer won’t stop until the timer occurs error (for example, being canceled).

In this PR, it will stop the timer task after the thread perf_counter_number_percentile_atomic is in is terminated, we need consider whether this change haves an effect on the result of timer task for calc(ctx) (in line 420).

What does it mean "whether this change haves an effect on the result of timer task for calc(ctx)"? Can you write a unit test? I don't see any negative effect of removing add_ref/release_ref.

src/core/perf_counter/perf_counter_atomic.h

neverchanje · 2020-02-04T01:43:58Z

src/core/perf_counter/perf_counter_atomic.h

-                                            std::placeholders::_1));
-            }
+            boost::shared_ptr<compute_context> ctx(new compute_context());
+            calc(ctx);


Suggested change

calc(ctx);

calc(boost::make_shared<compute_context>());

foreverneverer · 2020-02-04T03:09:21Z

Change & Potential problem

In the old version, after the thread perf_counter_number_percentile_atomic is in is terminated, the timer won’t stop until the timer occurs error (for example, being canceled).
In this PR, it will stop the timer task after the thread perf_counter_number_percentile_atomic is in is terminated, we need consider whether this change haves an effect on the result of timer task for calc(ctx) (in line 420).

What does it mean "whether this change haves an effect on the result of timer task for calc(ctx)"? Can you write a unit test?

No unit test, only point the "potential problem" for reviewing and expect your advice.

…a/rdsn into fix-mem_leak-perf_counter_atomic

Co-authored-by: Wu Tao <wutao1@xiaomi.com>

foreverneverer added 2 commits January 21, 2020 16:48

fix

fc3eacd

fix

fb394c0

levy5307 reviewed Jan 21, 2020

View reviewed changes

weekly-digest bot mentioned this pull request Jan 26, 2020

Weekly Digest (19 January, 2020 - 26 January, 2020) #385

Closed

neverchanje reviewed Feb 4, 2020

View reviewed changes

Merge branch 'master' into fix-mem_leak-perf_counter_atomic

1a6fd7b

foreverneverer added 2 commits February 4, 2020 13:46

update code

dcf17e1

Merge branch 'fix-mem_leak-perf_counter_atomic' of github.com:Shuo-Ji…

80947c7

…a/rdsn into fix-mem_leak-perf_counter_atomic

acelyc111 approved these changes Feb 4, 2020

View reviewed changes

neverchanje approved these changes Feb 4, 2020

View reviewed changes

foreverneverer merged commit d424088 into XiaoMi:master Feb 4, 2020

weekly-digest bot mentioned this pull request Feb 9, 2020

Weekly Digest (2 February, 2020 - 9 February, 2020) #389

Closed

neverchanje mentioned this pull request Mar 30, 2020

Release 1.12.3 apache/incubator-pegasus#506

Closed

neverchanje pushed a commit that referenced this pull request Mar 31, 2020

fix(lsan): memory leak in perf_counter_atomic.h (#384)

c941d5a

Co-authored-by: Wu Tao <wutao1@xiaomi.com>

neverchanje added the 1.12.3 label Apr 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(lsan): memory leak in perf_counter_atomic.h #384

fix(lsan): memory leak in perf_counter_atomic.h #384

foreverneverer commented Jan 21, 2020 •

edited

Loading

levy5307 Jan 21, 2020

neverchanje Feb 4, 2020

foreverneverer Feb 4, 2020 •

edited

Loading

vagetablechicken commented Jan 21, 2020 •

edited

Loading

foreverneverer commented Jan 21, 2020

vagetablechicken commented Jan 22, 2020

foreverneverer commented Jan 22, 2020

Change & Potential problem

neverchanje left a comment •

edited

Loading

neverchanje Feb 4, 2020

foreverneverer commented Feb 4, 2020

Change & Potential problem

	_timer.reset(new boost::asio::deadline_timer(tools::shared_io_service::instance().ios));
	_timer->expires_from_now(
	boost::posix_time::seconds(rand() % _counter_computation_interval_seconds + 1));
	this->add_ref();
	_timer->async_wait(std::bind(
	&perf_counter_number_percentile_atomic::on_timer, this, _timer, std::placeholders::_1));
	}

	~perf_counter_number_percentile_atomic(void) { _timer->cancel(); }

fix(lsan): memory leak in perf_counter_atomic.h #384

fix(lsan): memory leak in perf_counter_atomic.h #384

Conversation

foreverneverer commented Jan 21, 2020 • edited Loading

Coredump

Reason & Solution

Change & Potential problem

levy5307 Jan 21, 2020

Choose a reason for hiding this comment

neverchanje Feb 4, 2020

Choose a reason for hiding this comment

foreverneverer Feb 4, 2020 • edited Loading

Choose a reason for hiding this comment

vagetablechicken commented Jan 21, 2020 • edited Loading

foreverneverer commented Jan 21, 2020

vagetablechicken commented Jan 22, 2020

foreverneverer commented Jan 22, 2020

Change & Potential problem

neverchanje left a comment • edited Loading

Choose a reason for hiding this comment

Change & Potential problem

neverchanje Feb 4, 2020

Choose a reason for hiding this comment

foreverneverer commented Feb 4, 2020

Change & Potential problem

foreverneverer commented Jan 21, 2020 •

edited

Loading

foreverneverer Feb 4, 2020 •

edited

Loading

vagetablechicken commented Jan 21, 2020 •

edited

Loading

neverchanje left a comment •

edited

Loading