From 1ce152f59747decfffad5d5fc2ad7e3a90c4ad88 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 21 Mar 2020 20:18:50 +0800 Subject: [PATCH] crimson/admin: do not reset connected_sock before closing * no need to discard_result(). as `output_stream::close()` returns an empty future<> already * free the connected socket after the background task finishes, because: we should not free the connected socket before the promise referencing it is fulfilled. otherwise we have error messages from ASan, like ==287182==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000019aa0 at pc 0x55e2ae2de882 bp 0x7fff7e2bf080 sp 0x7fff7e2bf078 READ of size 8 at 0x611000019aa0 thread T0 #0 0x55e2ae2de881 in seastar::reactor_backend_aio::await_events(int, __sigset_t const*) ../src/seastar/src/core/reactor_backend.cc:396 #1 0x55e2ae2dfb59 in seastar::reactor_backend_aio::reap_kernel_completions() ../src/seastar/src/core/reactor_backend.cc:428 #2 0x55e2adbea397 in seastar::reactor::reap_kernel_completions_pollfn::poll() (/var/ssd/ceph/build/bin/crimson-osd+0x155e9397) #3 0x55e2adaec6d0 in seastar::reactor::poll_once() ../src/seastar/src/core/reactor.cc:2789 #4 0x55e2adae7cf7 in operator() ../src/seastar/src/core/reactor.cc:2687 #5 0x55e2adb7c595 in __invoke_impl&> /usr/include/c++/10/bits/invoke.h:60 #6 0x55e2adb699b0 in __invoke_r&> /usr/include/c++/10/bits/invoke.h:113 #7 0x55e2adb50222 in _M_invoke /usr/include/c++/10/bits/std_function.h:291 #8 0x55e2adc2ba00 in std::function::operator()() const /usr/include/c++/10/bits/std_function.h:622 #9 0x55e2adaea491 in seastar::reactor::run() ../src/seastar/src/core/reactor.cc:2713 #10 0x55e2ad98f1c7 in seastar::app_template::run_deprecated(int, char**, std::function&&) ../src/seastar/src/core/app-template.cc:199 #11 0x55e2a9e57538 in main ../src/crimson/osd/main.cc:148 #12 0x7fae7f20de0a in __libc_start_main ../csu/libc-start.c:308 #13 0x55e2a9d431e9 in _start (/var/ssd/ceph/build/bin/crimson-osd+0x117421e9) 0x611000019aa0 is located 96 bytes inside of 240-byte region [0x611000019a40,0x611000019b30) freed by thread T0 here: #0 0x7fae80a4e487 in operator delete(void*, unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xac487) #1 0x55e2ae302a0a in seastar::aio_pollable_fd_state::~aio_pollable_fd_state() ../src/seastar/src/core/reactor_backend.cc:458 #2 0x55e2ae2e1059 in seastar::reactor_backend_aio::forget(seastar::pollable_fd_state&) ../src/seastar/src/core/reactor_backend.cc:524 #3 0x55e2adab9b9a in seastar::pollable_fd_state::forget() ../src/seastar/src/core/reactor.cc:1396 #4 0x55e2adab9d05 in seastar::intrusive_ptr_release(seastar::pollable_fd_state*) ../src/seastar/src/core/reactor.cc:1401 #5 0x55e2ace1b72b in boost::intrusive_ptr::~intrusive_ptr() /opt/ceph/include/boost/smart_ptr/intrusive_ptr.hpp:98 #6 0x55e2ace115a5 in seastar::pollable_fd::~pollable_fd() ../src/seastar/include/seastar/core/internal/pollable_fd.hh:109 #7 0x55e2ae0ed35c in seastar::net::posix_server_socket_impl::~posix_server_socket_impl() ../src/seastar/include/seastar/net/posix-stack.hh:161 #8 0x55e2ae0ed3cf in seastar::net::posix_server_socket_impl::~posix_server_socket_impl() ../src/seastar/include/seastar/net/posix-stack.hh:161 #9 0x55e2ae0ed943 in std::default_delete::operator()(seastar::net::api_v2::server_socket_impl*) const /usr/include/c++/10/bits/unique_ptr.h:81 #10 0x55e2ae0db357 in std::unique_ptr >::~unique_ptr() /usr/include/c++/10/bits/unique_ptr.h:357 #11 0x55e2ae1438b7 in seastar::api_v2::server_socket::~server_socket() ../src/seastar/src/net/stack.cc:195 #12 0x55e2aa1c7656 in std::_Optional_payload_base::_M_destroy() /usr/include/c++/10/optional:260 #13 0x55e2aa16c84b in std::_Optional_payload_base::_M_reset() /usr/include/c++/10/optional:280 #14 0x55e2ac24b2b7 in std::_Optional_base_impl >::_M_reset() /usr/include/c++/10/optional:432 #15 0x55e2ac23f37b in std::optional::reset() /usr/include/c++/10/optional:975 #16 0x55e2ac21a2e7 in crimson::admin::AdminSocket::stop() ../src/crimson/admin/admin_socket.cc:265 #17 0x55e2aa099825 in operator() ../src/crimson/osd/osd.cc:450 #18 0x55e2aa0d4e3e in apply ../src/seastar/include/seastar/core/apply.hh:36 Signed-off-by: Kefu Chai --- src/crimson/admin/admin_socket.cc | 19 +++++++++---------- src/crimson/admin/admin_socket.h | 1 + 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/crimson/admin/admin_socket.cc b/src/crimson/admin/admin_socket.cc index cdb8e31d1558..307ea6b033a8 100644 --- a/src/crimson/admin/admin_socket.cc +++ b/src/crimson/admin/admin_socket.cc @@ -220,8 +220,7 @@ seastar::future<> AdminSocket::handle_client(seastar::input_stream& in, return in.close(); }).handle_exception([](auto ep) { logger().debug("exception on {}: {}", __func__, ep); - return seastar::make_ready_future<>(); - }).discard_result(); + }); } seastar::future<> AdminSocket::start(const std::string& path) @@ -236,7 +235,7 @@ seastar::future<> AdminSocket::start(const std::string& path) auto sock_path = seastar::socket_address{ seastar::unix_domain_addr{ path } }; server_sock = seastar::engine().listen(sock_path); // listen in background - std::ignore = seastar::do_until( + task = seastar::do_until( [this] { return stop_gate.is_closed(); }, [this] { return seastar::with_gate(stop_gate, [this] { @@ -257,11 +256,7 @@ seastar::future<> AdminSocket::start(const std::string& path) } }); }); - }).then([] { - logger().debug("AdminSocket::init(): admin-sock thread terminated"); - return seastar::now(); }); - return seastar::make_ready_future<>(); } @@ -271,13 +266,17 @@ seastar::future<> AdminSocket::stop() return seastar::now(); } server_sock->abort_accept(); - server_sock.reset(); if (connected_sock) { connected_sock->shutdown_input(); connected_sock->shutdown_output(); - connected_sock.reset(); } - return stop_gate.close(); + return stop_gate.close().then([this] { + assert(task.has_value()); + return task->then([] { + logger().info("AdminSocket: stopped"); + return seastar::now(); + }); + }); } ///////////////////////////////////////// diff --git a/src/crimson/admin/admin_socket.h b/src/crimson/admin/admin_socket.h index a81508e3fc6b..f2c10621ea71 100644 --- a/src/crimson/admin/admin_socket.h +++ b/src/crimson/admin/admin_socket.h @@ -152,6 +152,7 @@ private: seastar::future execute_command(const std::vector& cmd, ceph::bufferlist&& buf); + std::optional> task; std::optional server_sock; std::optional connected_sock; -- 2.47.3