Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

fix(perf_counter): remove static map from counter_info #735

Merged
merged 4 commits into from
Jan 25, 2021

Conversation

zhangyifan27
Copy link
Contributor

@zhangyifan27 zhangyifan27 commented Jan 21, 2021

In latest tests I found that pegasus server crashed when destroy a static map after called exit() in libc.so.6.
Coredump stack is:

(gdb) bt
#0  std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, dsn::tools::perf_counter_ptr_type>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, dsn::tools::perf_counter_ptr_type> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, dsn::tools::perf_counter_ptr_type> > >::_M_erase (this=this@entry=0x7f1ab9634cc0 <dsn::tools::counter_info::pointer_type[abi:cxx11]>, 
    __x=0x434548435f424452) at /home/zhangyifan8/local/gcc-5.4.0/include/c++/5.4.0/bits/stl_tree.h:1612
#1  0x00007f1ab922f61c in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, dsn::tools::perf_counter_ptr_type>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, dsn::tools::perf_counter_ptr_type> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, dsn::tools::perf_counter_ptr_type> > >::_M_erase (this=0x7f1ab9634cc0 <dsn::tools::counter_info::pointer_type[abi:cxx11]>, 
    __x=0x3236eb0) at /home/zhangyifan8/local/gcc-5.4.0/include/c++/5.4.0/bits/stl_tree.h:1612
#2  0x00007f1ab4678dba in __cxa_finalize () from /lib64/libc.so.6
#3  0x00007f1ab85006f6 in __do_global_dtors_aux () from /home/work/app/pegasus/c4tst-hdfs/meta/package/bin/libdsn_meta_server.so
#4  0x00007f1aba771548 in ?? ()
#5  0x0000000000000001 in ?? ()
#6  0x00007f1a352e99f0 in ?? ()

To avoid this we should follow google style guide

Objects with static storage duration are forbidden unless they are trivially destructible. [1]

I removed the static map from counter_info, and loop through the array counter_info_ptr to implement the method find_counter_type. Because we only had 11 elements(each element has two keys) in the array, this may not lead to much performance degradation.

[1] https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables

@neverchanje neverchanje added the type/sanitize Fixes on errors reported by sanitizers. label Jan 22, 2021
@hycdong hycdong merged commit d71b517 into XiaoMi:master Jan 25, 2021
zhangyifan27 added a commit to zhangyifan27/rdsn that referenced this pull request Jan 26, 2021
zhangyifan27 added a commit to zhangyifan27/rdsn that referenced this pull request Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/sanitize Fixes on errors reported by sanitizers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants