Crash due to incorrect object destruction order

[zyfjeff]

Title: Crash due to incorrect object destruction order

Description:

I am doing secondary development based on envoy to support the Dubbo protocol. I have a crash problem. I found that ConnectionImpl will destruct filter_manager_ when destructuring, and filter_manager_ will destruct the Dubbo filter I wrote. When Dubbo filter is destructed, ConnectionImpl::fd is called, and this function uses the socket_ of ConnectionImpl, but socket_ has been destructed beforefilter_manager_.

https://github.com/envoyproxy/envoy/blob/master/source/common/network/connection_impl.h#L137

IMO: The filter_manager_ should be destructed earlier than socket_.
I don't know if my understanding is correct. If it is not correct, please give me some suggestions.

Call Stack:

(gdb) bt
#0  0x00000000009d3c82 in Envoy::Network::ConnectionImpl::fd (this=0x3585500) at bazel-out/k8-dbg/bin/source/common/network/_virtual_includes/connection_lib/common/network/connection_impl.h:106
#1  <function called from gdb>
#2  0x00000000009d3c82 in Envoy::Network::ConnectionImpl::fd (this=0x3585500) at bazel-out/k8-dbg/bin/source/common/network/_virtual_includes/connection_lib/common/network/connection_impl.h:106
#3  0x0000000000a83567 in Envoy::Network::ConnectionImpl::state (this=0x3585500) at source/common/network/connection_impl.cc:163
#4  0x00000000006a38a7 in Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager::sendLocalReply (this=0x3093ae0, metadata=..., response=..., end_stream=true) at source/extensions/filters/net
work/dubbo_proxy/conn_manager.cc:107
#5  0x000000000069544f in Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage::sendLocalReply (this=0x3145800, response=..., end_stream=true) at source/extensions/filters/network/dubbo_proxy/a
ctive_message.cc:249
#6  0x0000000000694266 in Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessageDecoderFilter::sendLocalReply (this=0x2f91a70, response=..., end_stream=true) at source/extensions/filters/network/
dubbo_proxy/active_message.cc:114
#7  0x000000000068c5dc in Envoy::Extensions::NetworkFilters::DubboProxy::Router::Router::UpstreamRequest::onResetStream (this=0x39cb620, reason=Envoy::Tcp::ConnectionPool::LocalConnectionFailure)
    at source/extensions/filters/network/dubbo_proxy/router/router_impl.cc:259
#8  0x000000000068b76c in Envoy::Extensions::NetworkFilters::DubboProxy::Router::Router::onEvent (this=0x300fb90, event=Envoy::Network::LocalClose) at source/extensions/filters/network/dubbo_proxy/rout
er/router_impl.cc:137
#9  0x0000000000cc370e in Envoy::Tcp::ConnPoolImpl::ActiveConn::onEvent (this=0x60fe500, event=Envoy::Network::LocalClose) at source/common/tcp/conn_pool.cc:436
#10 0x0000000000a84343 in Envoy::Network::ConnectionImpl::raiseEvent (this=0x35e7880, event=Envoy::Network::LocalClose) at source/common/network/connection_impl.cc:329
#11 0x0000000000a83755 in Envoy::Network::ConnectionImpl::closeSocket (this=0x35e7880, close_type=Envoy::Network::LocalClose) at source/common/network/connection_impl.cc:194
#12 0x0000000000a831a2 in Envoy::Network::ConnectionImpl::close (this=0x35e7880, type=Envoy::Network::NoFlush) at source/common/network/connection_impl.cc:110
#13 0x000000000068c01b in Envoy::Extensions::NetworkFilters::DubboProxy::Router::Router::UpstreamRequest::resetStream (this=0x39cb620) at source/extensions/filters/network/dubbo_proxy/router/router_imp
l.cc:195
#14 0x000000000068a61d in Envoy::Extensions::NetworkFilters::DubboProxy::Router::Router::onDestroy (this=0x300fb90) at source/extensions/filters/network/dubbo_proxy/router/router_impl.cc:17
#15 0x00000000006948a9 in Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage::~ActiveMessage (this=0x3145800, __in_chrg=<optimized out>) at source/extensions/filters/network/dubbo_proxy/activ
e_message.cc:148
#16 0x00000000006949fe in Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage::~ActiveMessage (this=0x3145800, __in_chrg=<optimized out>) at source/extensions/filters/network/dubbo_proxy/activ
e_message.cc:150#17 0x00000000006a4a8e in std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage>::operator() (this=0x3da7f30, __ptr=0x3145800) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linu
x-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:76#18 0x00000000006a41b3 in std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage> >::~unique_ptr
(this=0x3da7f30, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:239#19 0x00000000006a4eb6 in std::_List_node<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage>
 > >::~_List_node (this=0x3da7f20, __in_chrg=<optimized out>)    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/stl_list.h:106
#20 0x00000000006a4ed6 in __gnu_cxx::new_allocator<std::_List_node<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::D
ubboProxy::ActiveMessage> > > >::destroy<std::_List_node<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy:
:ActiveMessage> > > > (this=0x3093b28, __p=0x3da7f20)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/ext/new_allocator.h:124
#21 0x00000000006a49d2 in std::__cxx11::_List_base<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::Activ
eMessage> >, std::allocator<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage> > > >::_M_clear (this=0x3093b28) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/list.tcc:75
#22 0x00000000006a3ffc in std::__cxx11::_List_base<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage> >, std::allocator<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage> > > >::~_List_base (this=0x3093b28, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/stl_list.h:446
#23 0x00000000006a3ed4 in std::__cxx11::list<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage> >, std::allocator<std::unique_ptr<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage, std::default_delete<Envoy::Extensions::NetworkFilters::DubboProxy::ActiveMessage> > > >::~list (this=0x3093b28, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/stl_list.h:507
#24 0x00000000006a2ea4 in Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager::~ConnectionManager (this=0x3093ae0, __in_chrg=<optimized out>, __vtt_parm=<optimized out>)
    at bazel-out/k8-dbg/bin/source/extensions/filters/network/dubbo_proxy/_virtual_includes/conn_manager_lib/extensions/filters/network/dubbo_proxy/conn_manager.h:30
#25 0x00000000006867b7 in __gnu_cxx::new_allocator<Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager>::destroy<Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager> (this=0x3093ae0, __p=0x3093ae0)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/ext/new_allocator.h:124
#26 0x0000000000686483 in std::allocator_traits<std::allocator<Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager> >::destroy<Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager> (__a=..., __p=0x3093ae0)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/alloc_traits.h:542
#27 0x00000000006860c1 in std::_Sp_counted_ptr_inplace<Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager, std::allocator<Envoy::Extensions::NetworkFilters::DubboProxy::ConnectionManager>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x3093ad0)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/shared_ptr_base.h:531
#28 0x00000000004205b2 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x3093ad0) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/shared_ptr_base.h:150
#29 0x000000000041f5c1 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x33ad828, __in_chrg=<optimized out>) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/shared_ptr_base.h:659
#30 0x000000000067e6f8 in std::__shared_ptr<Envoy::Network::ReadFilter, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x33ad820, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/shared_ptr_base.h:925
#31 0x000000000067e714 in std::shared_ptr<Envoy::Network::ReadFilter>::~shared_ptr (this=0x33ad820, __in_chrg=<optimized out>) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/shared_ptr.h:93
#32 0x0000000000a8d8a8 in Envoy::Network::FilterManagerImpl::ActiveReadFilter::~ActiveReadFilter (this=0x33ad800, __in_chrg=<optimized out>) at bazel-out/k8-dbg/bin/source/common/network/_virtual_includes/filter_manager_lib/common/network/filter_manager_impl.h:52
#33 0x0000000000a8d8e8 in Envoy::Network::FilterManagerImpl::ActiveReadFilter::~ActiveReadFilter (this=0x33ad800, __in_chrg=<optimized out>) at bazel-out/k8-dbg/bin/source/common/network/_virtual_includes/filter_manager_lib/common/network/filter_manager_impl.h:52
#34 0x0000000000a8ba9a in std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter>::operator() (this=0x3554bd0, __ptr=0x33ad800) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:76
#35 0x0000000000a8b4a5 in std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> >::~unique_ptr (this=0x3554bd0, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:239
#36 0x0000000000a8ab72 in std::_List_node<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> > >::~_List_node(this=0x3554bc0, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/stl_list.h:106
#37 0x0000000000a8ab92 in __gnu_cxx::new_allocator<std::_List_node<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> > > >::destroy<std::_List_node<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> > > > (this=0x3585538, __p=0x3554bc0) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/ext/new_allocator.h:124
#38 0x0000000000a890e6 in std::__cxx11::_List_base<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> >, std::allocator<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> > > >::_M_clear (this=0x3585538) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/list.tcc:75
#39 0x0000000000a884f8 in std::__cxx11::_List_base<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> >, std::allocator<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> > > >::~_List_base (this=0x3585538, __in_chrg=<optimized out>) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/stl_list.h:446
#40 0x0000000000a878c2 in std::__cxx11::list<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFilter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> >, std::allocator<std::unique_ptr<Envoy::Network::FilterManagerImpl::ActiveReadFil---Type <return> to continue, or q <return> to quit---
ter, std::default_delete<Envoy::Network::FilterManagerImpl::ActiveReadFilter> > > >::~list (this=0x3585538, __in_chrg=<optimized out>) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/stl_list.h:507
#41 0x0000000000a882a2 in Envoy::Network::FilterManagerImpl::~FilterManagerImpl (this=0x3585518, __in_chrg=<optimized out>) at bazel-out/k8-dbg/bin/source/common/network/_virtual_includes/filter_manager_lib/common/network/filter_manager_impl.h:39
#42 0x0000000000a82d61 in Envoy::Network::ConnectionImpl::~ConnectionImpl (this=0x3585500, __in_chrg=<optimized out>, __vtt_parm=<optimized out>) at source/common/network/connection_impl.cc:72

#43 0x0000000000a82e16 in Envoy::Network::ConnectionImpl::~ConnectionImpl (this=0x3585500, __in_chrg=<optimized out>, __vtt_parm=<optimized out>) at source/common/network/connection_impl.cc:81
#44 0x0000000000a6fba2 in std::default_delete<Envoy::Network::Connection>::operator() (this=0x3d054e8, __ptr=0x3585828) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:76
#45 0x0000000000a6e753 in std::unique_ptr<Envoy::Network::Connection, std::default_delete<Envoy::Network::Connection> >::~unique_ptr (this=0x3d054e8, __in_chrg=<optimized out>)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:239
#46 0x0000000000a6bafd in Envoy::Server::ConnectionHandlerImpl::ActiveConnection::~ActiveConnection (this=0x3d054c0, __in_chrg=<optimized out>) at source/server/connection_handler_impl.cc:259

#47 0x0000000000a6bb56 in Envoy::Server::ConnectionHandlerImpl::ActiveConnection::~ActiveConnection (this=0x3d054c0, __in_chrg=<optimized out>) at source/server/connection_handler_impl.cc:263

#48 0x000000000048ece0 in std::default_delete<Envoy::Event::DeferredDeletable>::operator() (this=0x53f0710, __ptr=0x3d054c0) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:76
#49 0x0000000000a77151 in std::unique_ptr<Envoy::Event::DeferredDeletable, std::default_delete<Envoy::Event::DeferredDeletable> >::reset (this=0x53f0710, __p=0x3d054c0)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/bits/unique_ptr.h:347
#50 0x0000000000a74ed9 in Envoy::Event::DispatcherImpl::clearDeferredDeleteList (this=0x2ed0c60) at source/common/event/dispatcher_impl.cc:68
#51 0x0000000000a747dd in Envoy::Event::DispatcherImpl::<lambda()>::operator()(void) const (__closure=0x2ed0d98) at source/common/event/dispatcher_impl.cc:35
#52 0x0000000000a7626f in std::_Function_handler<void(), Envoy::Event::DispatcherImpl::DispatcherImpl(Envoy::Event::TimeSystem&, Envoy::Buffer::WatermarkFactoryPtr&&)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/functional:1871
q#53 0x00000000004f0406 in std::function<void ()>::operator()() const (this=0x2ed0d98) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/functional:2267
#54 0x0000000000a7c6c5 in _ZZN5Envoy5Event9TimerImplC4ERNS_9CSmartPtrI10event_baseXadL_Z15event_base_freeEEEESt8functionIFvvEEENKUlisPvE_clEisS9_ () at source/common/event/timer_impl.cc:14
#55 0x0000000000a7c6f4 in _ZZN5Envoy5Event9TimerImplC4ERNS_9CSmartPtrI10event_baseXadL_Z15event_base_freeEEEESt8functionIFvvEEENUlisPvE_4_FUNEisS9_ () at source/common/event/timer_impl.cc:14
#56 0x00000000010bc6c8 in event_process_active_single_queue (base=base@entry=0x2e3e840, max_to_process=max_to_process@entry=2147483647, endtime=endtime@entry=0x0, activeq=<optimized out>) at ../event.c:1646
#57 0x00000000010bcc2f in event_process_active (base=0x2e3e840) at ../event.c:1738
#58 event_base_loop (base=0x2e3e840, flags=0) at ../event.c:1961
#59 0x0000000000a75f86 in Envoy::Event::DispatcherImpl::run (this=0x2ed0c60, type=Envoy::Event::Dispatcher::Block) at source/common/event/dispatcher_impl.cc:161
#60 0x0000000000a67b9b in Envoy::Server::WorkerImpl::threadRoutine (this=0x2f3e4e0, guard_dog=...) at source/server/worker_impl.cc:101
#61 0x0000000000a6752e in Envoy::Server::WorkerImpl::<lambda()>::operator()(void) const (__closure=0x2f3fda0) at source/server/worker_impl.cc:74
#62 0x0000000000a68bfb in std::_Function_handler<void(), Envoy::Server::WorkerImpl::start(Envoy::Server::GuardDog&)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...)
    at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/functional:1871
#63 0x00000000004f0406 in std::function<void ()>::operator()() const (this=0x2f3fda0) at /usr/local/gcc54/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../../include/c++/5.5.0/functional:2267
#64 0x00000000010c61b2 in _ZZN5Envoy6Thread6ThreadC4ESt8functionIFvvEEENKUlPvE_clES5_ () at source/common/common/thread.cc:21
#65 0x00000000010c61d7 in _ZZN5Envoy6Thread6ThreadC4ESt8functionIFvvEEENUlPvE_4_FUNES5_ () at source/common/common/thread.cc:23
#66 0x00007ffff71b0e25 in start_thread () from /lib64/libpthread.so.0
#67 0x00007ffff6bdb34d in clone () from /lib64/libc.so.6

[mattklein123]

@zyfjeff yeah this sounds like an order of destruction bug. I can't think of any reason not to rearrange. Can you fix?

[zyfjeff]

@zyfjeff No problem, I will fix it.

[klarose]

Do we also want to change the Buffer stuff in ConnectionImpl as well? Ultimately the reason for the issue in ordering is that we have a bit of a circular dependency -- filter_manager_ depends on this, which means that there isn't a nice construction order that would guarantee this type of issue is impossible. All we can do is ensure that the portions of this that _filter_manager_ depends on are declared prior to it. One of those is socket_. The other is anything used by ConnectionImpl's BufferSource interface, which I think is read_* and current_write_*

[zyfjeff]

The current implementation of FilterManager_ does not call the BufferSource related interface when destructuring, so I think the problem is not big at present.