Order of object destruction in wspp_callback_client causes crash

[dejaniv]

The client object is destroyed before connection object causing valgrind to report invalid read which leads to crash.

Valgrind output can be reproduced by attempting to connect to to non-existing endpoint.

The crash is happening if there are multiple attempts to connect to non-existing endpoint together with extensive data allocation/deallocation happening on heap simultaneously (on another thread). The callstack is the same as invalid read's callstack.

Possible fix is to move declaration of m_con after m_client in wspp_callback_client class in file Release/src/websockets/client/ws_client_wspp.cpp.

Additionally the client should not be released before wspp_callback_client is destroyed to prevent race condition. The following lines should be removed from the same file:

             // Delete client to make sure Websocketpp cleans up all Boost.Asio portions.
            m_client.reset();

Valgrind output:

==1== Invalid read of size 8
==1==    at 0x61BBCD2: boost::asio::detail::reactive_socket_service_base::destroy(boost::asio::detail::reactive_socket_service_base::base_implementation_type&) (reactive_socket_service_base.ipp:89)
==1==    by 0x6259295: boost::asio::basic_io_object<boost::asio::detail::reactive_socket_service<boost::asio::ip::tcp>, true>::~basic_io_object() (basic_io_object.hpp:247)
==1==    by 0x624CD33: boost::asio::basic_socket<boost::asio::ip::tcp>::~basic_socket() (basic_socket.hpp:1742)
==1==    by 0x624CD61: boost::asio::basic_stream_socket<boost::asio::ip::tcp>::~basic_stream_socket() (basic_stream_socket.hpp:219)
==1==    by 0x62B5549: boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >::~stream() (stream.hpp:121)
==1==    by 0x62B39E7: void __gnu_cxx::new_allocator<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > >::destroy<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > >(boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >*) (new_allocator.h:140)
==1==    by 0x62B0E50: void std::allocator_traits<std::allocator<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > > >::destroy<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > >(std::allocator<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > >&, boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >*) (alloc_traits.h:487)
==1==    by 0x62A9180: std::_Sp_counted_ptr_inplace<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >, std::allocator<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > >, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:554)
==1==    by 0x4DFE52F: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:155)
==1==    by 0x4DF7E8E: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:728)
==1==    by 0x61C99D3: std::__shared_ptr<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> >, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1167)
==1==    by 0x61C99EF: std::shared_ptr<boost::asio::ssl::stream<boost::asio::basic_stream_socket<boost::asio::ip::tcp> > >::~shared_ptr() (shared_ptr.h:103)
==1==    by 0x6268CCB: websocketpp::transport::asio::tls_socket::connection::~connection() (tls.hpp:63)
==1==    by 0x6268DB5: websocketpp::transport::asio::connection<websocketpp::config::asio_tls_client::transport_config>::~connection() (connection.hpp:67)
==1==    by 0x62B2BF7: websocketpp::connection<websocketpp::config::asio_tls_client>::~connection() (connection.hpp:235)
==1==    by 0x62B2C17: void __gnu_cxx::new_allocator<websocketpp::connection<websocketpp::config::asio_tls_client> >::destroy<websocketpp::connection<websocketpp::config::asio_tls_client> >(websocketpp::connection<websocketpp::config::asio_tls_client>*) (new_allocator.h:140)
==1==    by 0x62AD422: void std::allocator_traits<std::allocator<websocketpp::connection<websocketpp::config::asio_tls_client> > >::destroy<websocketpp::connection<websocketpp::config::asio_tls_client> >(std::allocator<websocketpp::connection<websocketpp::config::asio_tls_client> >&, websocketpp::connection<websocketpp::config::asio_tls_client>*) (alloc_traits.h:487)
==1==    by 0x629E57E: std::_Sp_counted_ptr_inplace<websocketpp::connection<websocketpp::config::asio_tls_client>, std::allocator<websocketpp::connection<websocketpp::config::asio_tls_client> >, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:554)
==1==    by 0x4DFE52F: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:155)
==1==    by 0x4DF7E8E: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:728)
==1==    by 0x5639132: std::__shared_ptr<void, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1167)
==1==    by 0x5639170: std::shared_ptr<void>::~shared_ptr() (shared_ptr.h:103)
==1==    by 0x61CB011: web::websockets::client::details::wspp_callback_client::~wspp_callback_client() (ws_client_wspp.cpp:141)
==1==    by 0x62B37F0: void __gnu_cxx::new_allocator<web::websockets::client::details::wspp_callback_client>::destroy<web::websockets::client::details::wspp_callback_client>(web::websockets::client::details::wspp_callback_client*) (new_allocator.h:140)
==1==    by 0x62B0D46: void std::allocator_traits<std::allocator<web::websockets::client::details::wspp_callback_client> >::destroy<web::websockets::client::details::wspp_callback_client>(std::allocator<web::websockets::client::details::wspp_callback_client>&, web::websockets::client::details::wspp_callback_client*) (alloc_traits.h:487)
==1==    by 0x62A8CF0: std::_Sp_counted_ptr_inplace<web::websockets::client::details::wspp_callback_client, std::allocator<web::websockets::client::details::wspp_callback_client>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:554)
==1==    by 0x4DFE52F: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:155)
==1==    by 0x4DF7E8E: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:728)
==1==    by 0x5681546: std::__shared_ptr<web::websockets::client::details::websocket_client_callback_impl, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1167)
==1==    by 0x5681584: std::shared_ptr<web::websockets::client::details::websocket_client_callback_impl>::~shared_ptr() (shared_ptr.h:103)
==1==    by 0x56815C2: web::websockets::client::websocket_callback_client::~websocket_callback_client() (ws_client.h:507)
==1==    by 0x5681604: void __gnu_cxx::new_allocator<web::websockets::client::websocket_callback_client>::destroy<web::websockets::client::websocket_callback_client>(web::websockets::client::websocket_callback_client*) (new_allocator.h:140)
==1==    by 0x567DAA4: void std::allocator_traits<std::allocator<web::websockets::client::websocket_callback_client> >::destroy<web::websockets::client::websocket_callback_client>(std::allocator<web::websockets::client::websocket_callback_client>&, web::websockets::client::websocket_callback_client*) (alloc_traits.h:487)
==1==    by 0x5677AED: std::_Sp_counted_ptr_inplace<web::websockets::client::websocket_callback_client, std::allocator<web::websockets::client::websocket_callback_client>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:554)
==1==    by 0x4DFE52F: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:155)
==1==    by 0x4DF7E8E: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:728)
==1==    by 0x563474E: std::__shared_ptr<web::websockets::client::websocket_callback_client, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1167)
==1==    by 0x5642A05: std::__shared_ptr<web::websockets::client::websocket_callback_client, (__gnu_cxx::_Lock_policy)2>::operator=(std::__shared_ptr<web::websockets::client::websocket_callback_client, (__gnu_cxx::_Lock_policy)2>&&) (shared_ptr_base.h:1263)
==1==    by 0x563A92C: std::shared_ptr<web::websockets::client::websocket_callback_client>::operator=(std::shared_ptr<web::websockets::client::websocket_callback_client>&&) (shared_ptr.h:335)
...
std::thread::_State_impl<std::thread::_Invoker<std::tuple<QueueingSenderPool::start()::{lambda()#1}> > >::_M_run() (thread:196)
==1==    by 0x4FB56B7: ??? (in /usr/lib/libstdc++.so.6.0.25)
==1==    by 0x4050F81: ??? (in /lib/ld-musl-x86_64.so.1)
==1==    by 0x6618FFF: ???
==1==    by 0x405304C: ??? (in /lib/ld-musl-x86_64.so.1)
==1==    by 0x4FB56AB: ??? (in /usr/lib/libstdc++.so.6.0.25)
==1==    by 0x495E1DF: ???
==1==  Address 0x67fb7d0 is 48 bytes inside a block of size 56 free'd
==1==    at 0x489CBDF: operator delete(void*) (vg_replace_malloc.c:586)
==1==    by 0x629CAB3: boost::asio::detail::reactive_socket_service<boost::asio::ip::tcp>::~reactive_socket_service() (reactive_socket_service.hpp:48)
==1==    by 0x543C1AE: boost::asio::detail::service_registry::destroy(boost::asio::execution_context::service*) (service_registry.ipp:110)
==1==    by 0x543C14F: boost::asio::detail::service_registry::destroy_services() (service_registry.ipp:54)
==1==    by 0x543C2A9: boost::asio::execution_context::destroy() (execution_context.ipp:46)
==1==    by 0x543C1F9: boost::asio::execution_context::~execution_context() (execution_context.ipp:35)
==1==    by 0x61B6EE5: boost::asio::io_context::~io_context() (io_context.ipp:55)
==1==    by 0x61DE456: websocketpp::transport::asio::endpoint<websocketpp::config::asio_tls_client::transport_config>::~endpoint() (endpoint.hpp:113)
==1==    by 0x61D3765: websocketpp::endpoint<websocketpp::connection<websocketpp::config::asio_tls_client>, websocketpp::config::asio_tls_client>::~endpoint() (endpoint.hpp:112)
==1==    by 0x61CB133: websocketpp::client<websocketpp::config::asio_tls_client>::~client() (client_endpoint.hpp:47)
==1==    by 0x61CD3A7: web::websockets::client::details::wspp_callback_client::websocketpp_tls_client::~websocketpp_tls_client() (ws_client_wspp.cpp:757)
==1==    by 0x61CD3CF: web::websockets::client::details::wspp_callback_client::websocketpp_tls_client::~websocketpp_tls_client() (ws_client_wspp.cpp:757)
==1==    by 0x61DDBBB: std::default_delete<web::websockets::client::details::wspp_callback_client::websocketpp_client_base>::operator()(web::websockets::client::details::wspp_callback_client::websocketpp_client_base*) const (unique_ptr.h:81)
==1==    by 0x61D33D6: std::unique_ptr<web::websockets::client::details::wspp_callback_client::websocketpp_client_base, std::default_delete<web::websockets::client::details::wspp_callback_client::websocketpp_client_base> >::~unique_ptr() (unique_ptr.h:274)
==1==    by 0x61CAFD2: web::websockets::client::details::wspp_callback_client::~wspp_callback_client() (ws_client_wspp.cpp:141)
==1==    by 0x62B37F0: void __gnu_cxx::new_allocator<web::websockets::client::details::wspp_callback_client>::destroy<web::websockets::client::details::wspp_callback_client>(web::websockets::client::details::wspp_callback_client*) (new_allocator.h:140)
==1==    by 0x62B0D46: void std::allocator_traits<std::allocator<web::websockets::client::details::wspp_callback_client> >::destroy<web::websockets::client::details::wspp_callback_client>(std::allocator<web::websockets::client::details::wspp_callback_client>&, web::websockets::client::details::wspp_callback_client*) (alloc_traits.h:487)
==1==    by 0x62A8CF0: std::_Sp_counted_ptr_inplace<web::websockets::client::details::wspp_callback_client, std::allocator<web::websockets::client::details::wspp_callback_client>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:554)
==1==    by 0x4DFE52F: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:155)
==1==    by 0x4DF7E8E: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:728)
==1==    by 0x5681546: std::__shared_ptr<web::websockets::client::details::websocket_client_callback_impl, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1167)
==1==    by 0x5681584: std::shared_ptr<web::websockets::client::details::websocket_client_callback_impl>::~shared_ptr() (shared_ptr.h:103)
==1==    by 0x56815C2: web::websockets::client::websocket_callback_client::~websocket_callback_client() (ws_client.h:507)
==1==    by 0x5681604: void __gnu_cxx::new_allocator<web::websockets::client::websocket_callback_client>::destroy<web::websockets::client::websocket_callback_client>(web::websockets::client::websocket_callback_client*) (new_allocator.h:140)
==1==    by 0x567DAA4: void std::allocator_traits<std::allocator<web::websockets::client::websocket_callback_client> >::destroy<web::websockets::client::websocket_callback_client>(std::allocator<web::websockets::client::websocket_callback_client>&, web::websockets::client::websocket_callback_client*) (alloc_traits.h:487)
==1==    by 0x5677AED: std::_Sp_counted_ptr_inplace<web::websockets::client::websocket_callback_client, std::allocator<web::websockets::client::websocket_callback_client>, 
...