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<bool, seastar::reactor::run()::<lambda()>&> /usr/include/c++/10/bits/invoke.h:60
    #6 0x55e2adb699b0 in __invoke_r<bool, seastar::reactor::run()::<lambda()>&> /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<bool ()>::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<void ()>&&) ../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<seastar::pollable_fd_state>::~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<seastar::net::api_v2::server_socket_impl>::operator()(seastar::net::api_v2::server_socket_impl*) const /usr/include/c++/10/bits/unique_ptr.h:81
    #10 0x55e2ae0db357 in std::unique_ptr<seastar::net::api_v2::server_socket_impl, std::default_delete<seastar::net::api_v2::server_socket_impl> >::~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<seastar::api_v2::server_socket>::_M_destroy() /usr/include/c++/10/optional:260
    #13 0x55e2aa16c84b in std::_Optional_payload_base<seastar::api_v2::server_socket>::_M_reset() /usr/include/c++/10/optional:280
    #14 0x55e2ac24b2b7 in std::_Optional_base_impl<seastar::api_v2::server_socket, std::_Optional_base<seastar::api_v2::server_socket, false, false> >::_M_reset() /usr/include/c++/10/optional:432
    #15 0x55e2ac23f37b in std::optional<seastar::api_v2::server_socket>::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 <kchai@redhat.com>

diff --git a/src/crimson/admin/admin_socket.cc b/src/crimson/admin/admin_socket.cc
index cdb8e31d15580ca51e599c32025ce8eb8f333e91..307ea6b033a8241abcfef9b0126d30e75d2f3f2c 100644 (file)
--- 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<char>& 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 a81508e3fc6b41c48e62b5a0ebfba8b08b515f53..f2c10621ea712c05500ad3f5ce2d9704f152cd93 100644 (file)
--- a/src/crimson/admin/admin_socket.h
+++ b/src/crimson/admin/admin_socket.h
@@ -152,6 +152,7 @@ private:
   seastar::future<tell_result_t> execute_command(const std::vector<std::string>& cmd,
                                                 ceph::bufferlist&& buf);
 
+  std::optional<seastar::future<>> task;
   std::optional<seastar::server_socket> server_sock;
   std::optional<seastar::connected_socket> connected_sock;